Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distribute LLVM bitcode linker as a preview component #123423

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

kjetilkjeka
Copy link
Contributor

@kjetilkjeka kjetilkjeka commented Apr 3, 2024

The self-contained LLVM bitcode linker was recently added in #117458. It is currently only in use to link the Nvidia ptx assembly tests when running rustc tests (local or CI). In fact, the linker itself is currently only usable for the nvptx64-nvidia-cuda target, but more targets will be supported in the future.

The reason a new linker was needed for the ptx format is that the old one has not been updated the last few years. It worked fine for a while, but as LLVM changed it broke and the nvptx tests was disabled in rustc back in 2019. It was ad-hoc patched and have been used in a sub-optimal state by the community until now.

If this PR is merged, the LLVM bitcode linker will be distributed as a preview component that can be used as a replacement for the old ptx-linker for development in addition to rustc tests. In addition to installing the llvm-bitcode-linker component, also the llvm-tools component must be installed as the llvm-bitcode-linker works by calling llvm tools.

Even though the LLVM bitcode linker is in its early stages it already now provides a lot of value over the old ptx-linker just by working and using up-to-date llvm tooling. By shipping it as a component it will be easier to gather user experience and improving it.

@petrochenkov when installing as a component it will be installed in the self-contained folder and will not work with -Clink-self-contained=no (although for some reason I expect to be a bug -Clink-self-contained=-linker doesn't properly disable it). However, when building using x.py build it will be placed in the lib/rustlib/<target>/bin directory and will be available for internal tests even if -Clink-self-contained=no is passed.

CC: @Mark-Simulacrum as I very briefly discussed it with you some months ago https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/.E2.9C.94.20How.20to.20ship.20a.20new.20tool.20.28embedded.20linker.29.20to.20users.3F

@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2024

r? @clubby789

rustbot has assigned @clubby789.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 3, 2024
@rust-log-analyzer

This comment has been minimized.

@kjetilkjeka kjetilkjeka force-pushed the llvm_bitcode_linker_component branch from 9922d1e to a4ef243 Compare April 4, 2024 09:28
@clubby789
Copy link
Contributor

lgtm other than the typo. Since Mark originally suggested it I'm going to assign them to make sure this is the right procedure
r? @Mark-Simulacrum

@rustbot rustbot assigned Mark-Simulacrum and unassigned clubby789 Apr 6, 2024
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with commits squashed

@@ -64,6 +65,9 @@ impl OverlayKind {
"compiler/rustc_codegen_cranelift/LICENSE-APACHE",
"compiler/rustc_codegen_cranelift/LICENSE-MIT",
],
OverlayKind::LlvmBitcodeLinker => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a separate/new overlay kind rather than just using OverlayKind::Rust?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to provide the readme of the llvm-bitcode-linker. I have corrected the path to point to this readme file now.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2024
@kjetilkjeka kjetilkjeka force-pushed the llvm_bitcode_linker_component branch from 24e140b to 30e6af0 Compare April 15, 2024 12:59
@rustbot rustbot added the O-wasi Operating system: Wasi, Webassembly System Interface label Apr 15, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2024

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@kjetilkjeka kjetilkjeka force-pushed the llvm_bitcode_linker_component branch from 30e6af0 to 235d45e Compare April 15, 2024 13:39
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Apr 15, 2024

📌 Commit 235d45e has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 15, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 15, 2024
…mponent, r=Mark-Simulacrum

Distribute LLVM bitcode linker as a preview component

The self-contained LLVM bitcode linker was recently added in rust-lang#117458. It is currently only in use to link the Nvidia ptx assembly tests when running rustc tests (local or CI). In fact, the linker itself is currently only usable for the `nvptx64-nvidia-cuda` target, but more targets will be supported in the future.

The reason a new linker was needed for the ptx format is that the [old one](https://github.com/denzp/rust-ptx-linker) has not been updated the last few years. It worked fine for a while, but as LLVM changed it broke and the nvptx tests was [disabled in rustc back in 2019](rust-lang@f8f9a28). It was ad-hoc patched and have been used in a sub-optimal state by the community until now.

If this PR is merged, the LLVM bitcode linker will be distributed as a preview component that can be used as a replacement for the old ptx-linker for development in addition to rustc tests. In addition to installing the `llvm-bitcode-linker` component, also the `llvm-tools` component must be installed as the `llvm-bitcode-linker` works by calling llvm tools.

Even though the LLVM bitcode linker is in its early stages it already now provides a lot of value over the old ptx-linker just by working and using up-to-date llvm tooling. By shipping it as a component it will be easier to gather user experience and improving it.

`@petrochenkov` when installing as a component it will be installed in the self-contained folder and will not work with `-Clink-self-contained=no` (although for some reason I expect to be a bug `-Clink-self-contained=-linker` doesn't properly disable it). However, when building using `x.py build` it will be placed in the `lib/rustlib/<target>/bin` directory and will be available for internal tests even if `-Clink-self-contained=no` is passed.

CC: `@Mark-Simulacrum` as I very briefly discussed it with you some months ago https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/.E2.9C.94.20How.20to.20ship.20a.20new.20tool.20.28embedded.20linker.29.20to.20users.3F
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2024
Rollup of 12 pull requests

Successful merges:

 - rust-lang#123423 (Distribute LLVM bitcode linker as a preview component)
 - rust-lang#123548 (libtest: also measure time in Miri)
 - rust-lang#123666 (Fix some typos in doc)
 - rust-lang#123864 (Remove a HACK by instead inferring opaque types during expected/formal type checking)
 - rust-lang#123896 (Migrate some diagnostics in `rustc_resolve` to session diagnostic)
 - rust-lang#123919 (builtin-derive: tag → discriminant)
 - rust-lang#123922 (Remove magic constants when using `base_n`.)
 - rust-lang#123931 (Don't leak unnameable types in `-> _` recover)
 - rust-lang#123933 (move the LargeAssignments lint logic into its own file)
 - rust-lang#123934 (`rustc_data_structures::graph` mini refactor)
 - rust-lang#123941 (Fix UB in LLVM FFI when passing zero or >1 bundle)
 - rust-lang#123957 (disable create_dir_all_bare test on all(miri, windows))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e07d18f into rust-lang:master Apr 15, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 15, 2024
@bors
Copy link
Contributor

bors commented Apr 15, 2024

⌛ Testing commit 235d45e with merge 99d0186...

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2024
Rollup merge of rust-lang#123423 - kjetilkjeka:llvm_bitcode_linker_component, r=Mark-Simulacrum

Distribute LLVM bitcode linker as a preview component

The self-contained LLVM bitcode linker was recently added in rust-lang#117458. It is currently only in use to link the Nvidia ptx assembly tests when running rustc tests (local or CI). In fact, the linker itself is currently only usable for the `nvptx64-nvidia-cuda` target, but more targets will be supported in the future.

The reason a new linker was needed for the ptx format is that the [old one](https://github.com/denzp/rust-ptx-linker) has not been updated the last few years. It worked fine for a while, but as LLVM changed it broke and the nvptx tests was [disabled in rustc back in 2019](rust-lang@f8f9a28). It was ad-hoc patched and have been used in a sub-optimal state by the community until now.

If this PR is merged, the LLVM bitcode linker will be distributed as a preview component that can be used as a replacement for the old ptx-linker for development in addition to rustc tests. In addition to installing the `llvm-bitcode-linker` component, also the `llvm-tools` component must be installed as the `llvm-bitcode-linker` works by calling llvm tools.

Even though the LLVM bitcode linker is in its early stages it already now provides a lot of value over the old ptx-linker just by working and using up-to-date llvm tooling. By shipping it as a component it will be easier to gather user experience and improving it.

``@petrochenkov`` when installing as a component it will be installed in the self-contained folder and will not work with `-Clink-self-contained=no` (although for some reason I expect to be a bug `-Clink-self-contained=-linker` doesn't properly disable it). However, when building using `x.py build` it will be placed in the `lib/rustlib/<target>/bin` directory and will be available for internal tests even if `-Clink-self-contained=no` is passed.

CC: ``@Mark-Simulacrum`` as I very briefly discussed it with you some months ago https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/.E2.9C.94.20How.20to.20ship.20a.20new.20tool.20.28embedded.20linker.29.20to.20users.3F
@kjetilkjeka
Copy link
Contributor Author

kjetilkjeka commented Apr 16, 2024

When checking if this component made it into nightly today I'm running

$ rustup +nightly-2024-04-16 component list | grep llvm
llvm-tools-x86_64-unknown-linux-gnu

And the llvm-bitcode-linker is not picked up. I thought rustup would pick up on new available components. Is that wrong and something must be done in rustup as well?

Edit: I guess it's not rustup as I cannot find the component here as well https://static.rust-lang.org/dist/2024-04-16. Then I assume it must be something related to deployment scripts.

Edit2: It seems to me like the distribution script is running python3 x.py dist bootstrap --include-default-paths and copying files. This should include the llvm-bitcode-linker so I'm a bit confused to why it doesn't happen.

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 20, 2024
…ild_manifest, r=Mark-Simulacrum

Add llvm-bitcode-linker to build manifest

When creating rust-lang#123423 I didn't realize I also had to add the new component to the build-manifest. This PR finishes the work of adding it, by also adding it to the build manifest.

r? `@Mark-Simulacrum`
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 20, 2024
…ild_manifest, r=Mark-Simulacrum

Add llvm-bitcode-linker to build manifest

When creating rust-lang#123423 I didn't realize I also had to add the new component to the build-manifest. This PR finishes the work of adding it, by also adding it to the build manifest.

r? ``@Mark-Simulacrum``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2024
Rollup merge of rust-lang#124071 - kjetilkjeka:llvm_bitcode_linker_build_manifest, r=Mark-Simulacrum

Add llvm-bitcode-linker to build manifest

When creating rust-lang#123423 I didn't realize I also had to add the new component to the build-manifest. This PR finishes the work of adding it, by also adding it to the build manifest.

r? ``@Mark-Simulacrum``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-wasi Operating system: Wasi, Webassembly System Interface S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants