Bug 180625

Summary: Link-Time-Optimizations could be used
Product: WebKit Reporter: Walter Hüttenmeyer <walter.huettenmeyer>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WORKSFORME    
Severity: Enhancement CC: annulen, aperez, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, walter.huettenmeyer
Priority: P2    
Version: WebKit Local Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch to allow LTO
none
Patch to allow -flto=n; tested against webkit-gtk-2.24.2 none

Description Walter Hüttenmeyer 2017-12-09 13:26:32 PST
Created attachment 328914 [details]
Patch to allow LTO

Compiling webkitgtk+-2.18.3 locally on an AMD64 machine, it will fail when using --flto=x --fuse-linker-plugin.
Only way to compile it without errors was to disable LTO.

I found a small patch on the gentoo forums here: https://forums.gentoo.org/viewtopic-t-1052716-start-0.html (credits to user costel78).
Patching the source with this allows me to compile it fine with LTO enabled.

Could you include this in upstream, if it seems worth?
Comment 1 Konstantin Tokarev 2019-03-27 07:18:26 PDT
FWIW, Mac port uses clang's ThinLTO since r243396
Comment 2 Walter Hüttenmeyer 2019-07-02 13:04:54 PDT
Hi Konstantin,

thank you for the info. On Linux it still won't compile with "-flto=n"
the attached patch does do the magic.

I do not know if any of the changes are in the correct and appropriate sections,
and even it's not included a statement like "well, you could do that without harming much" may do to include it in the gentoo tree until you decide on what to do with the patch.
Comment 3 Walter Hüttenmeyer 2019-07-02 13:07:24 PDT
Created attachment 373354 [details]
Patch to allow -flto=n; tested against webkit-gtk-2.24.2
Comment 4 Konstantin Tokarev 2019-07-02 13:16:57 PDT
Comment on attachment 373354 [details]
Patch to allow -flto=n; tested against webkit-gtk-2.24.2

View in context: https://bugs.webkit.org/attachment.cgi?id=373354&action=review

> b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp   2016-01-14 21:25:52.000000000 +0100:30
> +#if __GNUC__ >= 5

We require GCC 7 now so this check is redundant
Comment 5 Adrian Perez 2022-02-28 15:05:12 PST
LTO has always worked fine for me without this patch. For the sake of
being thorough, I just did a build using the following command for
configuring it:

  CC=gcc CXX=g++ \
  LDFLAGS='-flto=27 -fuse-linker-plugin -fuse-ld=bfd' \
  CFLAGS='-flto=27 -fuse-linker-plugin' \
  CXX='g++ -flto=27 -fuse-linker-plugin' \
  cmake -S. -Bbuild-gtk -DCMAKE_BUILD_TYPE=Release -DPORT=GTK -GNinja && \
  ninja -Cbuild-gtk -j27

This completed just fine :)

I am going to close this as WORKSFORME, but if somebody runs into the
same issue again of course we can reopen it. In that case, please do
indicate how to reproduce the build issue (commands used for building,
and information about the compiler/linker/etc. being used).
Comment 6 Adrian Perez 2022-02-28 15:07:51 PST
For the record, I also tried Clang/LLD with -flto=thin/-fuse-ld=lld
and WebKitGTK built fine as well. Compiler versions here were:

  % gcc -dumpversion                                                  
  11.2.0
  % clang -dumpversion                                            
  13.0.1