LLVM Bitcode Linker: A self contained linker for nvptx and other targets

[kjetilkjeka]

This PR introduces a new linker named llvm-bitcode-linker. It is a self-contained linker that can be used to link programs in llbc before optimizing and compiling to native code. It will first be used internally in the Rust compiler to enable tests for the nvptx64-nvidia-cuda target as the original rust-ptx-linker is deprecated. It will then be provided to users of the nvptx64-nvidia-cuda target with the purpose of linking ptx. More targets than nvptx will also be supported eventually.

The PR introduces a new unstable LinkerFlavor for the compiler. The compiler will also not be shipped with rustc but most likely instead be shipped in it's own unstable component (a follow up PR will be opened for this). This means that merging this PR should not add any stability guarantees.

When more details of self-contained is implemented it will only be possible to use the linker when -Clink-self-contained=+linker is passed.

Original Description

When this PR was created it was focused a bit differently. The original text is preserved here in case there's some interests in it

I have experimenting with approaches to replace the ptx-linker and enable the nvptx target tests again. I think it's time to get some feedback on the approach.

The problem

The only useful linker for the nvptx target is this crate. Since this linker performs linking on llvm bitcode it needs to track the llvm version of rustc and use the same format. It has not been maintained for 3+ years and must be considered abandoned. Over the years rust have upgraded LLVM while the linker has been left to bitrot. It is no longer in a usable state.

Due to the difficulty of keeping the ptx-linker up to date outside of tree the nvptx tests was disabled a long time ago. It was previously discussed if adding the ptx-linker to the rust repo would be a possibility. My efforts in doing this stopped at getting an answered if the license would prohibit it from inclusion in the Rust repo. I therefore concluded that a re-write would be necessary.

The possible solution presented here

The llvm tools know perfectly well how to link and optimize llvm bitcode. Each of them only perform a single task, and are therefore a bit cumbersome to call with the current linker approach rustc takes.

This PR adds a simple tool (current name embedded-linker) which can link self contained (often embedded) programs in llvm bitcode before compiling to the target format. Optimization will also be performed if lto is enabled. The rust compiler will make a single invocation to this tool, while the tool will orchestrate the many calls to the llvm tools.

The questions

• Is having control over the nvptx linking and therefore also tests worth it to add such tool? or should the tool live outside the rust repo?
• Is the approach of calling llvm tools acceptable? Or would we want to keep the ptx-linker approach of using the llvm library? The tools seems to provide more simplicity and stability, but more intermediate files are being written. Perhaps there also are some performance penalty for the calling tools approach.
• What is the process for adding such tool? MCP?
• Does adding llvm-link to the llvm-tool component require any process?
• Does it require some sort of FCP to remove ptx-linker as the default linker for ptx? Or is it sufficient that using the upstream ptx-linker is broken in its current state. it is possible to use a somewhat patched version of ptx-linker.

[rustbot]

r? @oli-obk

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

[kjetilkjeka]

@rustbot label +O-NVPTX

[oli-obk]

cc @nagisa and @Zoxc tor previous involvement

[kjetilkjeka]

Cc: @workingjubilee as I remember we talked about this in this issue denzp/rust-ptx-linker#34 (comment)

[bjorn3]

Can we require fat LTO for nvptx? When rustc does fat LTO it already combines all crates into a single object file.

[juntyr]

I'm really excited to hear that PTX might gain a maintained linker again!

Two years ago I was using Rust to write single-source kernels for NVIDIA GPUs, where LTO and inlining were crucial. I at some point had to fork the ptx-linker crate to keep LTO working with newer LLVM versions. It would be awesome if linking would "just work" instead again, so that the linker's is kept in sync and supports fat LTO.

Thank you so much for working on this ❤️

[kjetilkjeka]

Can we require fat LTO for nvptx? When rustc does fat LTO it already combines all crates into a single object file.

It would certainly be possible, rust is capable of calling the right functions in llvm to make it work. But it doesn't work "out of the box" and I'm not certain it's the correct thing to do long term eiteher.

If we decided to use only lto="fat" we would disallow lto="thin" which helps a lot with compile times. One could argue that the general ptx program is small enough for this to be acceptable, but it doesn't feel right to leave such an optimization out without an excellent reason. Perhaps a bigger problem is that linker-plugin-lto would not work which would make cross-language lto more difficult.

When it comes to compiler change we would still need to add llc as a linker known to the compiler. This is required to compile from llvm-bitcode to native. The linker would be an unconventional one as it would not actually be able to link anything and only accept a single bitcode file as input.

When I tested it just now it seems like libcompiler_builtins still produces an .rlib which is linked by the linker even when lto = "fat". I'm not sure if this is a bug or expected behavior, but it must be resolved before we may assume that the linker is only given a single bitcode file. (Note: -Zbuild-std=core is required due to not distributing this library and LTO towards libcore is required to work around some bugs right now and desired at the long term since calling functions on ptx is tremendously expensive.)

As an addition, there might be future benefits an approach as this embedded-linker can have to other targets than ptx as well. Maybe it can be used to bring thin lto to embedded targets where the native linker doesn't support it. It can also provide compilation without requiring to install an external target specific linker.

Is this an approach we would like to explore further even with the known deficiencies listed above? Is the main advantage that we avoid yet another tool that needs to be maintained? Or that we get more control using libLLVM compared to calling the llvm tools?

[bjorn3]

When I tested it just now it seems like libcompiler_builtins still produces an .rlib which is linked by the linker even when lto = "fat".

That will be fixed soon. There is an open PR for making compiler-builtins participate in LTO.

[kjetilkjeka]

There are a few questions in my original post, but I think the one that stops me from moving forward is figuring out whether this is an approach that can be accepted. Are there any processes required to get something like this MR approved (MCP?) or is it just a matter of completing the implementation?

Insight like "this is probably not going to be accepted, but bjorn3's alternative probably will be" would also be very helpful to help me avoid putting effort into something that is probably not going to go anywhere.

[petrochenkov]

I planned to review it in a couple of weeks.
Generally, if the new tool is interface-compatible with the previous ptx-linker then it doesn't need a new linker flavor, it can reuse the old one.
Also, whether tools are looked up inside the rustc toolchain (e.g. the new linker) or elsewhere (e.g. the old ptx-linker) is controlled by the -C link-self-contained option, you may check how it works as well.

[petrochenkov]

I think the general approach is fine, we can ship a small additional tool, especially if it's not enabled by default.
The linker just needs to reuse the old PTX linker flavor and the linker's location should be determined through -C link-self-contained, as I said in the comment above.
@rustbot author

[petrochenkov]

This should be ready now.
Thanks!
@bors r+

[bors]

📌 Commit 8b47f5a has been approved by petrochenkov

It is now in the queue for this repository.

[kjetilkjeka]

I did a manual rebase to fix the conflict now. It was only a minor conflict (in change_trackers.rs) so this one should still be ready.

@rustbot label -S-waiting-on-author +S-waiting-on-review

[petrochenkov]

@bors r+

[bors]

📌 Commit 355e833 has been approved by petrochenkov

It is now in the queue for this repository.

[rustbot]

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

• 0995767

[kjetilkjeka]

@petrochenkov Yet another conflict in change_trackers.rs resolved.

@rustbot label -S-waiting-on-author +S-waiting-on-review

[petrochenkov]

@bors r-

[petrochenkov]

@bors r+

[bors]

📌 Commit 843dd28 has been approved by petrochenkov

It is now in the queue for this repository.