Bug 188986 – [CMake] Add support for LTO builds

• Status: RESOLVED
• Resolution: FIXED
• Component: CMake

[Don Olmstead]

Like https://bugs.webkit.org/show_bug.cgi?id=187785 but in CMake form

[Christopher Reid]

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.

[Don Olmstead]

Comment on attachment 368700 [details]
Patch

r=me assuming bots are happy.

[Keith Rollin]

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.

[Christopher Reid]

(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.

[Michael Catanzaro]

(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.

[Christopher Reid]

(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.

[Yusuke Suzuki]

(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

[EWS Watchlist]

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

[EWS Watchlist]

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

[WebKit Commit Bot]

Comment on attachment 368700 [details]
Patch

Clearing flags on attachment: 368700

Committed r244895: <https://trac.webkit.org/changeset/244895>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.