Skip to content

Fixup and re-enable many tests for NVPTX#157512

Open
kulst wants to merge 4 commits into
rust-lang:mainfrom
kulst:nvptx-fixup-tests
Open

Fixup and re-enable many tests for NVPTX#157512
kulst wants to merge 4 commits into
rust-lang:mainfrom
kulst:nvptx-fixup-tests

Conversation

@kulst

@kulst kulst commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

While working with NVPTX, I found that many of its tests are currently disabled. They are configured as only-nvptx64 and ignore-nvptx64 at the same time, so they did not run at all. (For tests-suites not running in CI for NVPTX only-nvptx64 alone is sufficient to prevent a test from running)
Some of the tests also failed when I forced them to run.

I would like to improve the test coverage for NVPTX with this PR as a starting point. It

  • fixes and re-enables some tests for NVPTX
  • adds minicore, where needed.
  • adds linker-asm as an assembly-output directive for linkers that emit assembly by default (like llbc) or which can be explicitly instructed to do so.

This PR also adds to the rustc-dev-guide best-practices for target-specific tests and a warning mentioning the interactions of only-X directives with our CI.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
r? @ZuseZ4

@rustbot

rustbot commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. O-NVPTX Target: the NVPTX LLVM backend for running rust on GPUs, https://llvm.org/docs/NVPTXUsage.html labels Jun 5, 2026
@ZuseZ4

ZuseZ4 commented Jun 5, 2026

Copy link
Copy Markdown
Member

Nice find. I'm very busy till June 12th, but happy to check then if you don't mind waiting a bit, otherwise please feel free to reroll.

@kulst
kulst force-pushed the nvptx-fixup-tests branch 2 times, most recently from f1256f5 to ceea353 Compare June 7, 2026 10:30
Comment thread tests/assembly-llvm/nvptx-safe-naming.rs Outdated
@ZuseZ4

ZuseZ4 commented Jun 24, 2026

Copy link
Copy Markdown
Member

re target-linker-default, do you maybe want to wait with that for a later PR where you'll actually use it in a test?
While going through the list, I also saw https://github.com/rust-lang/rust/blob/main/src/doc/rustc-dev-guide/src/tests/directives.md#assembly still mentioning the ptx-linker, can you add another commit to remove it, now that we dropped support for it?

@kulst
kulst force-pushed the nvptx-fixup-tests branch from ceea353 to dfee7f8 Compare July 1, 2026 12:29
@rustbot

rustbot commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

rustc-dev-guide is developed in its own repository. If possible, consider making this change to rust-lang/rustc-dev-guide instead.

cc @BoxyUwU, @tshepang

@rustbot rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label Jul 1, 2026
@rustbot

This comment has been minimized.

@kulst

kulst commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

re target-linker-default, do you maybe want to wait with that for a later PR where you'll actually use it in a test? While going through the list, I also saw https://github.com/rust-lang/rust/blob/main/src/doc/rustc-dev-guide/src/tests/directives.md#assembly still mentioning the ptx-linker, can you add another commit to remove it, now that we dropped support for it?

Not sure about that, since tests/assembly-llvm/nvptx-arch-default.rs and tests/assembly-llvm/nvptx-linking-cdylib.rs already use it?

My intention was to have an assembly-output directive, where the final linked artifact already is assembly. This allows assembly tests to consider linked code, too. This of course requires a linker that emits assembly by default or is at least able to when explicitly instructed to do so.
Considering this, probably linker-asm sounds more suitable.

I've rebased on top of the current main branch, added linker-asm to the documentation and removed ptx-linker from it.
Also I have enabled tests/assembly-llvm/nvptx-linking-cdylib.rs which is another test that requires the linker-asm directive.

@kulst
kulst force-pushed the nvptx-fixup-tests branch from dfee7f8 to 5e27b30 Compare July 1, 2026 13:45
@rustbot

This comment has been minimized.

@ZuseZ4

ZuseZ4 commented Jul 6, 2026

Copy link
Copy Markdown
Member

Thanks, the new name and note in the docs imho give a much better explanation of the change.
Let's verify

@bors try jobs=test-various

otherwise lgtm.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 6, 2026
Fixup and re-enable many tests for NVPTX


try-job: test-various
@rust-bors rust-bors Bot 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 Jul 7, 2026
@rust-bors

rust-bors Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

💔 Test for cfc5c92 failed: CI. Failed job:

@rust-log-analyzer

This comment has been minimized.

@kulst

kulst commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Hm, interesting.
x86_64-unknown-linux-musl overwrites the linker in CI. So tests that need llvm-bitcode-linker fail here. Maybe I could overwrite the linker in these tests by using -Clinker=llvm-bitcode-linker but not sure if this is the right approach. Probably making these tests only-nvptx64 would be better.
Another solution would be to make compiletest not specify -Clinker if a custom target is used.

Anyway, it made me look deeper into how our test infrastructure works.
Turns out, in CI only run-make, run-make-cargo and assembly-llvm test-suites are run with ./x.py test --target=nvptx64-nvidia-cuda exactly in test-various. So in all other test-suites only-nvptx64 leads to a test not being run in CI at all (like this one). This is a bit surprising to a user that writes a test.

I was tempted to simply not restrict which test-suites are run in CI targeting nvptx64. However, many tests in UI require std without specifying needs-target-std, which makes them already fail locally.

This leaves us with the current approach, but we probably should document it properly:

  • how only-nvptx64 works in the different test-suites
  • that especially UI tests must be written with --target=nvptx64-nvidia-cuda and not only-nvptx64

kulst added 2 commits July 10, 2026 11:42
If the linker supports to emit assembly as the final artifact, this
`assembly-output` directive can be used.
This allows assembly tests to also check the emitted assembly after linking.

The NVPTX target is the primary use case as this target only emits
assembly as its final artifact.
The `ptx-linker` is no longer used, but wasn't removed from the
rustc-dev-guide yet.
@kulst
kulst force-pushed the nvptx-fixup-tests branch from 5e27b30 to 883c79e Compare July 10, 2026 10:17
@rustbot

rustbot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@kulst
kulst force-pushed the nvptx-fixup-tests branch from 883c79e to 3f7c25f Compare July 10, 2026 10:20
@kulst

kulst commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

I created #159059 for the linker issue. This will allow us to stick to cross-build tests for NVPTX for all test-suites.

I also modified this PR a little:

  • removed unnecessary ignore-backends: gcc directives in codegen-llvm and assembly-llvm
  • added best-practices for target-specific tests to the dev guide
  • added a warning in the dev-guide mentioning the interactions of only-X directives with our CI
  • removed nvptx-linking-binary.rs, as --crate-type bin makes no sense for NVPTX

Since test-various does not run successfully without the linker fix:
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2026
@rust-log-analyzer

This comment has been minimized.

@kulst
kulst force-pushed the nvptx-fixup-tests branch 2 times, most recently from a6e3c49 to c26da7e Compare July 10, 2026 11:54
kulst added 2 commits July 10, 2026 13:56
A lot of tests for NVPTX were previously disabled. They were configured
as `only-nvptx` and `ignore-nvptx` at the same time, so they did not run at all.
they even did not succeed, even when forced to run.

Fixup and re-enable those tests to pass the test suite.

Add minicore, where needed.
Document that `only-X` directives can accidentally make tests unreachable in
CI when no CI job runs the corresponding test suite with matching conditions.

Also add guidance for target-specific tests: for targets that are not used as
hosts in CI, prefer cross-building the test to that target instead of making the
whole test `only-X` for that target.
@rust-bors

rust-bors Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 21, 2026
…rcote

Fix debuginfo argument when invoking LLBC linker

Previously LLBC's Linker implementation unconditionally added `--debug` to the linker invocation. This just became visible when 062ecd6 made LLBC no longer strip debug information unconditionally.

It is also common to use `cargo build --release` together with `-Zbuild-std=core` when targeting NVPTX. In this case the release profile will prevent all crates from embedding debug information in the first place. This additionally made the bug less visible.

This PR adds the proper logic to LLBC's linker implementation to only invoke LLBC with `--debug` when debug-information should not be stripped.

I did not add a regression test yet, since this would require to actually invoke the linker in `assembly-llvm`. As soon as rust-lang#157512 is merged and we have `assembly-output: linker-asm` available, I want to follow up on that.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 21, 2026
…rcote

Fix debuginfo argument when invoking LLBC linker

Previously LLBC's Linker implementation unconditionally added `--debug` to the linker invocation. This just became visible when 062ecd6 made LLBC no longer strip debug information unconditionally.

It is also common to use `cargo build --release` together with `-Zbuild-std=core` when targeting NVPTX. In this case the release profile will prevent all crates from embedding debug information in the first place. This additionally made the bug less visible.

This PR adds the proper logic to LLBC's linker implementation to only invoke LLBC with `--debug` when debug-information should not be stripped.

I did not add a regression test yet, since this would require to actually invoke the linker in `assembly-llvm`. As soon as rust-lang#157512 is merged and we have `assembly-output: linker-asm` available, I want to follow up on that.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 22, 2026
…rcote

Fix debuginfo argument when invoking LLBC linker

Previously LLBC's Linker implementation unconditionally added `--debug` to the linker invocation. This just became visible when 062ecd6 made LLBC no longer strip debug information unconditionally.

It is also common to use `cargo build --release` together with `-Zbuild-std=core` when targeting NVPTX. In this case the release profile will prevent all crates from embedding debug information in the first place. This additionally made the bug less visible.

This PR adds the proper logic to LLBC's linker implementation to only invoke LLBC with `--debug` when debug-information should not be stripped.

I did not add a regression test yet, since this would require to actually invoke the linker in `assembly-llvm`. As soon as rust-lang#157512 is merged and we have `assembly-output: linker-asm` available, I want to follow up on that.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 22, 2026
…rcote

Fix debuginfo argument when invoking LLBC linker

Previously LLBC's Linker implementation unconditionally added `--debug` to the linker invocation. This just became visible when 062ecd6 made LLBC no longer strip debug information unconditionally.

It is also common to use `cargo build --release` together with `-Zbuild-std=core` when targeting NVPTX. In this case the release profile will prevent all crates from embedding debug information in the first place. This additionally made the bug less visible.

This PR adds the proper logic to LLBC's linker implementation to only invoke LLBC with `--debug` when debug-information should not be stripped.

I did not add a regression test yet, since this would require to actually invoke the linker in `assembly-llvm`. As soon as rust-lang#157512 is merged and we have `assembly-output: linker-asm` available, I want to follow up on that.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
rust-timer added a commit that referenced this pull request Jul 22, 2026
Rollup merge of #159211 - kulst:llbc-fix-debuginfo, r=nnethercote

Fix debuginfo argument when invoking LLBC linker

Previously LLBC's Linker implementation unconditionally added `--debug` to the linker invocation. This just became visible when 062ecd6 made LLBC no longer strip debug information unconditionally.

It is also common to use `cargo build --release` together with `-Zbuild-std=core` when targeting NVPTX. In this case the release profile will prevent all crates from embedding debug information in the first place. This additionally made the bug less visible.

This PR adds the proper logic to LLBC's linker implementation to only invoke LLBC with `--debug` when debug-information should not be stripped.

I did not add a regression test yet, since this would require to actually invoke the linker in `assembly-llvm`. As soon as #157512 is merged and we have `assembly-output: linker-asm` available, I want to follow up on that.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
pull Bot pushed a commit to xtqqczze/rust-lang-miri that referenced this pull request Jul 22, 2026
Fix debuginfo argument when invoking LLBC linker

Previously LLBC's Linker implementation unconditionally added `--debug` to the linker invocation. This just became visible when 062ecd6 made LLBC no longer strip debug information unconditionally.

It is also common to use `cargo build --release` together with `-Zbuild-std=core` when targeting NVPTX. In this case the release profile will prevent all crates from embedding debug information in the first place. This additionally made the bug less visible.

This PR adds the proper logic to LLBC's linker implementation to only invoke LLBC with `--debug` when debug-information should not be stripped.

I did not add a regression test yet, since this would require to actually invoke the linker in `assembly-llvm`. As soon as rust-lang/rust#157512 is merged and we have `assembly-output: linker-asm` available, I want to follow up on that.

cc: @kjetilkjeka
@rustbot label +O-NVPTX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc O-NVPTX Target: the NVPTX LLVM backend for running rust on GPUs, https://llvm.org/docs/NVPTXUsage.html S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants