1475384 - Use lld-link for official WIndows builds

Status: RESOLVED FIXED

Description

[David Major [:dmajor] (low availability)]

With the clang-cl transition going smoothly so far, the next step is to use lld-link as our linker.

Comment 1

[David Major [:dmajor] (low availability)]

Attachment: lld-link

Comment 2

[Gregory Szorc [:gps]]

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/

Comment 3

[David Major [:dmajor] (low availability)]

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.

Comment 4

[David Major [:dmajor] (low availability)]

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.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 6

[thakis]

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.

Comment 7

[thakis]

(Also, that bug is fixed if your lld is built at at least llvm r335864)

Comment 8

[thakis]

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

Comment 9

[David Major [:dmajor] (low availability)]

> 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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5650748284b
Use lld-link for official Windows builds. r=gps

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b5650748284b

Comment 12

[Jens Hausdorf :jens1o [ni? me]]

Are there any performance improvements caused by this?

Comment 13

[Ionuț Goldan [:igoldan], Performance Sheriffing]

(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