Stabilize linker-plugin based LTO (aka cross-language LTO)

[michaelwoerister]

This PR stabilizes linker plugin based LTO, also known as "cross-language LTO" because it allows for doing inlining and other optimizations across language boundaries in mixed Rust/C/C++ projects.

As described in the tracking issue, it works by making rustc emit LLVM bitcode instead of machine code, the same as clang does. A linker with the proper plugin (like LLD) can then run (Thin)LTO across all modules.

The feature has been implemented over a number of pull requests and there are various codegen and run-make tests that make sure that it keeps working.

It also works for building big projects like Firefox.

The PR makes the feature available under the -C linker-plugin-lto flag. As discussed in the tracking issue it is not cross-language specific and also not LLD specific. -C linker-plugin-lto is descriptive of what it does. If someone has a better name, let me know :)

[rust-highfive]

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

[michaelwoerister]

As discussed in the tracking issue, we'll ask @rust-lang/compiler to sign off on it.
@rfcbot fcp merge

[rfcbot]

Team member @michaelwoerister has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Zoxc
• @cramertj
• @eddyb
• @estebank
• @michaelwoerister
• @nagisa
• @nikomatsakis
• @oli-obk
• @petrochenkov
• @pnkfelix
• @varkor

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[michaelwoerister]

r? @alexcrichton

[alexcrichton]

Looks reasonable to me! Do you know if the way to use this is written down somewhere? I presume it's almost "just set RUSTFLAGS with this flag" but we may want to mention compatible linkers and such? (also things like LLVM version compatibility, etc)

[bors]

☔ The latest upstream changes (presumably #57937) made this pull request unmergeable. Please resolve the merge conflicts.

[michaelwoerister]

Do you know if the way to use this is written down somewhere?

Yeah, I ran into a similar problem with path-prefix-remapping a while back. There seems to be no canonical place for documenting niche-features like these. Should I create a page on https://forge.rust-lang.org/? @rust-lang/docs might have a better suggestion.

[mark-i-m]

The reference?

[michaelwoerister]

The reference looks like it's more about the language, not about the compiler.

[michaelwoerister]

O, I just found this: https://doc.rust-lang.org/rustc/index.html

It looks a bit unmaintained though...

[steveklabnik]

It looks a bit unmaintained though...

I knocked out the basics, and we'd like to continue to expand it, but just need people to do the work, as always :)

It was created to exactly be the place for this kind of information.

[michaelwoerister]

OK, I'll just add a subsection for the feature. It's complicated enough to warrant its own page, I think.

[Zoxc]

@michaelwoerister Is there any mechanism to ensure matching LLVM IR versions are used?

I feel like this option should be -C lto, but that's already used for other things. It also seems to me that this doesn't have to be implemented by a plugin, so that probably shouldn't be a part of the option, unless you're actually passing a plugin path for the linker to use.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[michaelwoerister]

Is there any mechanism to ensure matching LLVM IR versions are used?

What do you have in mind? Only the linker sees both versions of the LLVM IR, so don't think that rustc can do a check unfortunately.

It also seems to me that this doesn't have to be implemented by a plugin, so that probably shouldn't be a part of the option, unless you're actually passing a plugin path for the linker to use.

I'm open for suggestions regarding a different name for the command line option. I don't like -C linker-lto because that sounds weird if you know what "LTO" stands for. The name xLTO has been used for the feature by some folks but I think it's suitable for a commandline option. -C defer-lto would be kind of OK, but -C linker-plugin-lto has more useful information in it. As even LLD uses -plugin for its CLI here, I think it's fine.

[jamesmunns]

Are there any plans to integrate this as a cargo flag? Or is this intended to stay as a RUSTFLAG?

This would be great to have on in most cases for embedded, especially if this would inline external assembly files as well.

Edit: specifically as a profile flag that could be set in a Cargo.toml, not a cargo CLI flag

[gnzlbg]

It also works for building big projects like Firefox.

Is Firefox using it to compile its jemalloc-sys fork somewhere ?

[michaelwoerister]

Are there any plans to integrate this as a cargo flag?

Not that I'm aware of. @rust-lang/cargo would have to decide about this.

Is Firefox using it to compile its jemalloc-sys fork somewhere ?

Mabye Ted knows something about this? cc @luser

[alexcrichton]

@jamesmunns FWIW this unfortunately wouldn't work for external assembly files, linker-based LTO only works for code compiled by the same compiler to the same IR, such as C and Rust compiled both to LLVM IR. External assembly files are compiled directly to object files and skip the LLVM IR step, so they can't be optimized/inlined together.

I think we'll probably want to stabilize this first and then see how it plays out to figure out how to best add it to Cargo

[gnzlbg]

I think we'll probably want to stabilize this first and then see how it plays out to figure out how to best add it to Cargo

It might be best to add it to the cc crate first. A lot of crates use cc in their build.rs to either compile C/C++ directly, or to obtain "Makefile" options that are then passed to autoconf/automake, make, cmake, and friends.

That is, we probably want cc to detect whether the C compiler that will / should be used is the clang binary shipped with Rust, and if that's the case, then detect whether the linker is the appropriate one. If all stars align, then the cc crate should automatically use this. We probably want to be able to enforce it through some configuration option, which could then be enabled also externally e.g. via an environment variable that users and cargo could set.

[alexcrichton]

r=me when this is green on CI

[cramertj]

@bors r=alexcrichton

[bors]

📌 Commit 3a9d171 has been approved by alexcrichton

[joshtriplett]

First of all, this is awesome, and I'd love to see it happen.

Second, while I wouldn't want to block this on it, we need to take this change into account in any future designs for better ways of handling builds of hybrid C/Rust projects.

[glandium]

I would like to bring to your attention that stabilizing this requires the LLVM version used in Rust to be a non-arbitrary version of LLVM trunk. It does happen that LLVM IR breaks compatibility on trunk (and it ends up being fixed at some point), and IIRC, it has happened that Rust picked a version of LLVM that happened to be broken.

[gnzlbg]

I would like to bring to your attention that stabilizing this requires the LLVM version used in Rust to be a non-arbitrary version of LLVM trunk.

Why? This option is disabled by default. If a user enables it, and the LLVM used is incompatible, we could:

• error
• emit a warning and disable the option (compilation proceeds)
• disable the option without error or warning (compilation proceeds silently).

AFAICT, this option enables and optimization and that can be "best-effort". We should document the behavior, and that's it.

If the current behavior is neither of these (e.g. unsound, an LLVM error is emitted, etc.), you can open an issue about this. That's bad and should be fixed.

[nikic]

@glandium While this is true in general, rust is currently using (and has been since its release) the LLVM 8 branch, not trunk. If you're seeing issues, they cannot be related to trunk changes.

[gnzlbg]

@nikic users might be using the system LLVM (up to 6.0), depending on their platform (linux distribution, etc.). This case needs to be handled somehow.

[nikic]

@gnzlbg That's true. Using the system rust (which will likely be linked against system LLVM) should work though. We also have #56371 which suggests to ship a compatible clang in rustup.

[ghost]

@michaelwoerister -flto=full never works