[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Emacs build fails [MSYS2/MINGW64]
From: |
Paul Eggert |
Subject: |
Re: Emacs build fails [MSYS2/MINGW64] |
Date: |
Mon, 29 Apr 2019 12:38:02 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 4/29/19 7:53 AM, Eli Zaretskii wrote:
> see, for example, the comments by Florian
> Weimer in this GCC bug report:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88793
Those comments don't invalidate all uses of __attribute__ ((cold)) in
Emacs. Florian says that in cold functions on some platforms, strlen on
large (5000-byte) strings can be as much as 60 times slower due to
questionable optimizations in GCC that are currently being rethought
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88809#c5>. However, if the
functions currently marked 'cold' in Emacs are called so rarely (at
least on such large strings) that the performance cost (factor of 60 for
rarely-used code) is outweighed by the performance benefit (small factor
of improvement in commonly-used code), then from a performance viewpoint
it's still preferable to mark these functions 'cold'.
> the GCC manual indicates that marking a function as 'cold'
> causes that function to be optimized for size, and we already know
> that GCC 8.x has at least one bug with -Os used together with -O2,
The GCC bug report <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90020>
says that the bug can also occur with -O2, and can occur without
__attribute__ ((cold)). So it's not as simple as saying "__attribute__
((cold)) is buggy".
> I think we have way too many functions marked with this
> attribute. Functions like wrong_type_argument or Fthrow or
> Fabort_recursive_edit or Fsignal or error or Ftop_level -- do they
> really fit the below description?
It should be purely a performance issue.
For example, on my platform (x86-64, GCC 8.3.1, -O2, Fedora 29) after
inlining, Emacs has 1066 static calls to wrong_type_argument, and with
__attribute__ ((cold)) all these calling sites can be optimized. In
bytecode.c this causes GCC to move wrong_type_argument calls to a cold
section of the text, which means the bytecode interpreter should run
faster for all the usual branch-prediction and icache reasons.
I measured the effect of __attribute__ ((cold)) (including both costs
and benefits) as improving overall performance of a big benchmark ('make
compile-always') by 1.3% on my platform. This test included all the
roughly 50 functions currently marked as cold in Emacs. Although I can
imagine that removing __attribute__ ((cold)) from some of these
functions would not hurt overall performance significantly, I didn't
think it worth the time to investigate which ones. That is, I didn't
investigate marking a smaller set of functions. If someone wants to do
that work then I expect that we should be able to remove markings from
some of these functions without affecting performance significantly.
> if we leave it in the
> sources, we should disable it for MinGW, it seems.
Yes, that sounds like a good idea. I installed the attached patch. It
would be good to know what the actual bug is (it's possible that it's
not a compiler bug at all, but is a portability bug in Emacs), but
that's less urgent.
0001-Disable-__attribute__-cold-on-MinGW.patch
Description: Text Data
Re: Emacs build fails [MSYS2/MINGW64], Paul Eggert, 2019/04/28
- Re: Emacs build fails [MSYS2/MINGW64], Angelo Graziosi, 2019/04/28
- Re: Emacs build fails [MSYS2/MINGW64], Angelo Graziosi, 2019/04/29
- Re: Emacs build fails [MSYS2/MINGW64], martin rudalics, 2019/04/29
- Re: Emacs build fails [MSYS2/MINGW64], Eli Zaretskii, 2019/04/29
- Re: Emacs build fails [MSYS2/MINGW64],
Paul Eggert <=
- Re: Emacs build fails [MSYS2/MINGW64], Eli Zaretskii, 2019/04/30
- Re: Emacs build fails [MSYS2/MINGW64], Paul Eggert, 2019/04/30
- Re: Emacs build fails [MSYS2/MINGW64], Eli Zaretskii, 2019/04/30
Re: Emacs build fails [MSYS2/MINGW64], Stephen Leake, 2019/04/28