Bug 188986 - [CMake] Add support for LTO builds
Summary: [CMake] Add support for LTO builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CMake (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-27 10:20 PDT by Don Olmstead
Modified: 2019-05-02 16:39 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.92 KB, patch)
2019-05-01 13:40 PDT, Christopher Reid
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews210 for win-future (13.70 MB, application/zip)
2019-05-02 11:07 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2018-08-27 10:20:45 PDT
Like https://bugs.webkit.org/show_bug.cgi?id=187785 but in CMake form
Comment 1 Christopher Reid 2019-05-01 13:40:18 PDT
Created attachment 368700 [details]
Patch

Support enabling LTO when building with clang.
This patch only enables it for clang currently because GCC hits an internal error when compiled with -flto.

This patch tries to use ld.lld when LTO_MODE is specified since gold doesn't recognize llvm bitcode by default. The LLVMgold.so plugin would need to be used for building LTO with clang and gold.
Comment 2 Don Olmstead 2019-05-01 14:31:56 PDT
Comment on attachment 368700 [details]
Patch

r=me assuming bots are happy.
Comment 3 Keith Rollin 2019-05-01 15:43:09 PDT
I ran into some LTO issues when enabling it for Xcode, I want to make sure they are accounted for in your patch, too.

It's been a couple of years since I tried compiling JavaScriptCore with LTO, but when I did, I ran into a runtime crash. I don't understand the details, but the explanation I was given had something to do with JSC assuming there were implicit compiler fences that were abolished when LTO was applied. So if you're enabling LTO for JSC, be sure to run a full suite of tests. (If they pass, then I should probably try building JSC again under LTO to see if the bug I hit has gone away.)

Also, I had problems building for ARM and got an internal compiler error. I don't know if ARM is generally a target outside of Apple, so I don't know if I should assume you tested that configuration or not.

Finally, there were some issues with older clangs + lto.dylib's. In particular, there was a rare race condition that could lead to crashes at build time. I had to wait until Xcode 10.2 before I could actually turn on LTO. I don't know what the public versions of clang have on the inside, so I don't know how well-distributed the fix is. I could try to dig up some tool version information if this turns out to be something you want to account for. If you run `clang -v`, I might be able to find out if the fix is in there or not.
Comment 4 Christopher Reid 2019-05-01 17:57:00 PDT
(In reply to Keith Rollin from comment #3)

Thanks for letting us know what issues you've faced when turning on LTO.

> I ran into some LTO issues when enabling it for Xcode, I want to make sure
> they are accounted for in your patch, too.
> 
> It's been a couple of years since I tried compiling JavaScriptCore with LTO,
> but when I did, I ran into a runtime crash. I don't understand the details,
> but the explanation I was given had something to do with JSC assuming there
> were implicit compiler fences that were abolished when LTO was applied. So
> if you're enabling LTO for JSC, be sure to run a full suite of tests. (If
> they pass, then I should probably try building JSC again under LTO to see if
> the bug I hit has gone away.)
We're focused on enabling LTO without JIT on x86_64. I'm not sure if that fence issue was specific to JIT.
I also just tested jsc tests with jit enabled and haven't seen any new failures when turning on LTO on both clang-7 and clang-8 in my ubuntu environment.


> Also, I had problems building for ARM and got an internal compiler error. I
> don't know if ARM is generally a target outside of Apple, so I don't know if
> I should assume you tested that configuration or not.
I haven't looked at ARM support. Our platforms only target x86_64 but it would be good for Igalia to know if they want to support LTO in WPE.

> 
> Finally, there were some issues with older clangs + lto.dylib's. In
> particular, there was a rare race condition that could lead to crashes at
> build time. I had to wait until Xcode 10.2 before I could actually turn on
> LTO. I don't know what the public versions of clang have on the inside, so I
> don't know how well-distributed the fix is. I could try to dig up some tool
> version information if this turns out to be something you want to account
> for. If you run `clang -v`, I might be able to find out if the fix is in
> there or not.
I didn't seem to hit that race in my builds.
It would be nice to know if it's an issue in llvm 7 or llvm 8 since we're targeting those llvm versions for LTO.
Comment 5 Michael Catanzaro 2019-05-01 18:19:31 PDT
(In reply to Christopher Reid from comment #4)
> I haven't looked at ARM support. Our platforms only target x86_64 but it
> would be good for Igalia to know if they want to support LTO in WPE.

I think we do (if it works with GCC). Why go slower?

For LLVM, that's cool too, but nobody does production builds that way.
Comment 6 Christopher Reid 2019-05-01 18:24:05 PDT
(In reply to Christopher Reid from comment #4)
> (In reply to Keith Rollin from comment #3)
> I also just tested jsc tests with jit enabled and haven't seen any new
> failures when turning on LTO on both clang-7 and clang-8 in my ubuntu
> environment.

Sorry I might be mistaken about clang-7. It looks like there is a new test failing in clang-7 after turning on LTO `stress/json-stringify-string-builder-overflow.js.ftl-no-cjit-small-pool`. I haven't seen it fail in clang-8 with LTO. I'll have to check if it's specific to jit.
Comment 7 Yusuke Suzuki 2019-05-01 18:37:59 PDT
(In reply to Christopher Reid from comment #4)
> > I ran into some LTO issues when enabling it for Xcode, I want to make sure
> > they are accounted for in your patch, too.
> > 
> > It's been a couple of years since I tried compiling JavaScriptCore with LTO,
> > but when I did, I ran into a runtime crash. I don't understand the details,
> > but the explanation I was given had something to do with JSC assuming there
> > were implicit compiler fences that were abolished when LTO was applied. So
> > if you're enabling LTO for JSC, be sure to run a full suite of tests. (If
> > they pass, then I should probably try building JSC again under LTO to see if
> > the bug I hit has gone away.)
> We're focused on enabling LTO without JIT on x86_64. I'm not sure if that
> fence issue was specific to JIT.
> I also just tested jsc tests with jit enabled and haven't seen any new
> failures when turning on LTO on both clang-7 and clang-8 in my ubuntu
> environment.

I don't think this issue only happens in JIT enabled environment.
I think the typical issue is like this. http://trac.webkit.org/changeset/196259/webkit
Comment 8 EWS Watchlist 2019-05-02 11:07:27 PDT
Comment on attachment 368700 [details]
Patch

Attachment 368700 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12063404

New failing tests:
legacy-animation-engine/fast/layers/no-clipping-overflow-hidden-hardware-acceleration.html
Comment 9 EWS Watchlist 2019-05-02 11:07:35 PDT
Created attachment 368788 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 10 WebKit Commit Bot 2019-05-02 16:39:41 PDT
Comment on attachment 368700 [details]
Patch

Clearing flags on attachment: 368700

Committed r244895: <https://trac.webkit.org/changeset/244895>
Comment 11 WebKit Commit Bot 2019-05-02 16:39:42 PDT
All reviewed patches have been landed.  Closing bug.