Closed
Bug 1475384
Opened 6 years ago
Closed 6 years ago
Use lld-link for official WIndows builds
Categories
(Firefox Build System :: Toolchains, enhancement)
Firefox Build System
Toolchains
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: away, Assigned: away)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
760 bytes,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
With the clang-cl transition going smoothly so far, the next step is to use lld-link as our linker.
Assignee: nobody → dmajor
Attachment #8991749 -
Flags: review?(gps)
Comment 2•6 years ago
|
||
Comment on attachment 8991749 [details] [diff] [review] lld-link Review of attachment 8991749 [details] [diff] [review]: ----------------------------------------------------------------- I'll be honest: I'm not sure of the technical implications of this change. glandium would be a better reviewer on that front. But, you've been doing great work on the clang front and I trust that if you are asking for review everything is expected to mostly work and not cause major problems. If I'm wrong and this isn't a rubber stamp review, then please ask glandium for review. Regardless, \o/
Attachment #8991749 -
Flags: review?(gps) → review+
The riskiest thing about lld-link is going to be debug information, but with bug 1458109 finally sorted out I'm feeling much more confident in its quality.
Just to be safe I don't want the first Nightlies to go unmonitored over the weekend. I'll land this on Sunday for the Monday morning builds.
Updated•6 years ago
|
Blocks: linker-lld
Comment 5•6 years ago
|
||
Not sure how important this is, but Microsoft folks alerted us to an LLD flaw in CFI implementation that caused a crash for lld-linked Chrome in the MS x86 emulator, which is used to run win32 programs on arm64 devices: https://bugs.chromium.org/p/chromium/issues/detail?id=857012
I think that's very likely unrelated. This here isn't on an arm device, and that bug is about /cfguard not about stack probes too.
(Also, that bug is fixed if your lld is built at at least llvm r335864)
Sorry, ignore comment 6, I thought you had commented on bug 1475431. Comment 7 is relevant though: That bug has been fixed in lld since r335864
> That bug has been fixed in lld since r335864 Which we already have, hooray! https://searchfox.org/mozilla-central/source/build/build-clang/clang-win64.json
Comment 10•6 years ago
|
||
Pushed by dmajor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b5650748284b Use lld-link for official Windows builds. r=gps
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b5650748284b
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 12•6 years ago
|
||
Are there any performance improvements caused by this?
Flags: needinfo?(dmajor)
Comment 13•6 years ago
|
||
(In reply to Jens Hausdorf from comment #12) > Are there any performance improvements caused by this? Build times have reduced: == Change summary for alert #14358 (as of Sun, 15 Jul 2018 22:02:01 GMT) == Improvements: 7% build times windows2012-32-noopt debug taskcluster-c4.4xlarge 1,554.22 -> 1,451.30 5% build times windows2012-64-noopt debug taskcluster-c4.4xlarge 1,592.99 -> 1,511.48 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=14358
Updated•6 years ago
|
Flags: needinfo?(dmajor)
You need to log in
before you can comment on or make changes to this bug.
Description
•