Enable cross-language LTO on Win64

RESOLVED FIXED in Firefox 68

Status

enhancement
RESOLVED FIXED
6 months ago
5 days ago

People

(Reporter: mwoerister, Assigned: dmajor)

Tracking

(Blocks 1 bug)

unspecified
mozilla68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

6 months ago
Given the right combination of Clang and rustc, LLD can do cross-language ThinLTO. In order for this to work, Rust code must be built with the (currently still unstable) `-Z cross-lang-lto` flag. While testing and debugging the feature, I added this flag to `rustflags_override` in `config/rules.mk` (see
https://hg.mozilla.org/try/rev/b2592d085693e617e12d8ae936b70ad0ad01c85f). 

It would be great if cross-lang LTO could be easily enabled via `.mozconfig`. 

Some notes:

 - There might be additional places where the flag needs to be added. All Rust code that ends up in the Firefox distribution and is linked via LLD should have the flag. Tools or build scripts should *not* be compiled with the flag.

 - This build option will only work if the right combination of Clang, rustc, and LLD are used. In the future, when the flag is available in stable Rust, the build system might want to have a sanity check for this.

 - When the flag gets stabilized, it will look different (probably something like `-C xlto` or `-C lld-lto`). I'm not sure how to best handle this change.
I'm not particularly convinced it's limited to lld. BFD ld or gold, with the LLVM plugin, should just as well be able to do the linking.
Reporter

Comment 2

6 months ago
Yes, you are right, in theory gold and newer versions of ld support this too. In practice, I had some trouble though. ld never worked for me and gold seemed a bit unstable, while lld always worked fine. That being said, I haven't tried the other linkers in almost half a year now, so the situation might have improved.
We're doing thin lto with BFD ld on automation (also, Apple's ld64 for mac, where there's essentially no other choice), and haven't had problems with that. I don't expect linking more thin lto objects to make a difference.
Reporter

Comment 4

6 months ago
That's good to know. I also expect cross-lang-lto not to make a difference as long as the LLVM version of rustc, clang, and the linker plugin match closely enough.

Per conversation with :kmoir, I'm going through untriaged bugs in her components and marking the ones which look to be enhancements/tasks with the enhancement severity to get them out of the triage queue.

If this incorrect, please remove the tag.

Severity: normal → enhancement
Reporter

Comment 6

3 months ago

Eric asked me to write up the list of changes I had to make in order for things to compile with cross-lang LTO. For a local build on Windows the following .mozconfig works for me:

# Use the right compilers and linker:
export CC="clang-cl.exe"
export CXX="clang-cl.exe"
export LINKER="lld-link.exe"

# Enable cross-lang LTO
export RUSTFLAGS=-Clinker-plugin-lto

# Do a release build with ThinLTO enabled
ac_add_options --enable-release
ac_add_options --enable-lto=thin
ac_add_options --host=x86_64-pc-mingw32
ac_add_options --target=x86_64-pc-mingw32

# geckodriver seems to use `link.exe` regardless of the `LINKER` variable above.
# (--enable-linker does not seem to work on Windows?)
ac_add_options --disable-geckodriver

For testing I used clang 8.0 (~rc2?) from mach bootstrap and rustc 1.34.0-nightly (00aae71f5 2019-02-25).

geckodriver seems to be the only problem for local builds.

For things to work on CI, I also had to disable the clang plugin, as shown here:
https://hg.mozilla.org/try/rev/c57d3a67cb1d
I don't know if that is still necessary. It probably was also related to code being compiled with LTO but then linked with a linker that doesn't support it.

Assignee

Updated

3 months ago
Blocks: 1486042
Assignee

Comment 7

3 months ago
# geckodriver seems to use `link.exe` regardless of the `LINKER` variable
above.

glandium: cross-language LTO builds fail in geckodriver because they use link.exe which doesn't understand the bitcode format. I believe you pointed me to bug 1524396 in a different context the other day. Do you think that bug will fix the geckodriver LTO issue too?

Flags: needinfo?(mh+mozilla)

It shouldn't matter because it feels like we shouldn't be LTO'ing geckodriver in the first place.

Flags: needinfo?(mh+mozilla)
Assignee

Comment 9

3 months ago

(In reply to Mike Hommey [:glandium] from comment #8)

It shouldn't matter because it feels like we shouldn't be LTO'ing
geckodriver in the first place.

That means we'll have to do something more complicated than a global export RUSTFLAGS=-Clinker-plugin-lto. (Which means I'm probably not the right person to write the patch.)

Assignee

Updated

3 months ago
Depends on: 1533010

(In reply to David Major [:dmajor] from comment #9)

(In reply to Mike Hommey [:glandium] from comment #8)

It shouldn't matter because it feels like we shouldn't be LTO'ing
geckodriver in the first place.

That means we'll have to do something more complicated than a global export RUSTFLAGS=-Clinker-plugin-lto. (Which means I'm probably not the right person to write the patch.)

I think all this means is that you need to override RUSTFLAGS for force-cargo-library-{build,check} to include -Clinker-plugin-lto when appropriate. So you need to massage this code:

https://searchfox.org/mozilla-central/source/config/makefiles/rust.mk#129-133

to keep what's already there for not-libraries, and and the appropriate flags for libraries.

Updated

2 months ago
Depends on: 1538060

Updated

2 months ago
Depends on: 1524396

After talking things over with glandium it sounds like the linker issue has been handled in bug 1524396. We might want to a file a follow up to not LTO geckodriver (and others), but that's not a blocker. We need bug 1538060 for non-Windows builds (that should be landing shortly), but even without that we should be unblocked on Windows. There's a possibility that lib.exe issues might get in the way (bug 1537643) but it's probably worth resuming work on Windows to see if that's an issue or not.

Assignee

Comment 12

2 months ago

On Win64 builds that are already doing LTO (in practice, this means shippable builds), make the LTO be cross-language.

Attachment #9055291 - Attachment description: Enable cross-language LTO on Win64 → Enable cross-language LTO on Win64 shippable builds

Comment 13

2 months ago
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5851c65af0e0
Enable cross-language LTO on Win64 shippable builds r=glandium

Comment 14

2 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Summary: Support cross-language LTO in the build system → Enable cross-language LTO on Win64
Assignee: nobody → dmajor
No longer depends on: 1552329
You need to log in before you can comment on or make changes to this bug.