Convert -Ctarget-cpu into a target-modifier for AVR, AMDGCN and NVPTX #150732
Open
kulst wants to merge 2 commits into
Open
Convert -Ctarget-cpu into a target-modifier for AVR, AMDGCN and NVPTX #150732kulst wants to merge 2 commits into
-Ctarget-cpu into a target-modifier for AVR, AMDGCN and NVPTX #150732kulst wants to merge 2 commits into
Conversation
rustbot
added
S-waiting-on-author
kulst
changed the title
-Ctarget-cpu a target-modifier on NVPTX
Contributor
Author
|
@rustbot label: +O-NVPTX |
rustbot
added
the
O-NVPTX
kulst
changed the title
-Ctarget-cpu a target-modifier on NVPTX-Ctarget-cpu into a target-modifier on NVPTX
kulst
changed the title
-Ctarget-cpu into a target-modifier on NVPTX-Ctarget-cpu into a target-modifier for NVPTX
kulst
force-pushed
the
cpu_is_target_modifier
branch
from
7164067 to
a8f5e96
Compare
Collaborator
|
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb These commits modify compiler targets. |
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-waiting-on-author
Collaborator
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
Contributor
Author
|
r? @bjorn3 as you suggested this change. |
bjorn3
reviewed
Jan 8, 2026
Contributor
|
It works out really nice with -CTarget-cpu being able to be a target modifier. It would be nice to know what the plan was with the ptx isa version as well. I asked that question here |
Member
|
I can't vouch for the implementation, but the approach sounds good. :) Cc @Darksonn |
15 tasks
Contributor
Author
|
I have added some explanation of why |
Darksonn
reviewed
Jan 10, 2026
Darksonn
reviewed
Jan 10, 2026
Comment on lines
+128
to
+137
| pub(super) fn target_cpu( | ||
| sess: &Session, | ||
| l: &TargetModifier, | ||
| r: Option<&TargetModifier>, | ||
| ) -> bool { | ||
| if sess.target.cpu_is_target_modifier { | ||
| if let Some(r) = r { | ||
| return l.extend().tech_value == r.extend().tech_value; | ||
| } else { | ||
| return false; |
Member
There was a problem hiding this comment.
What happens if -Ctarget-cpu appears multiple times on the command line? Please add a test.
Contributor
Author
There was a problem hiding this comment.
The value of the last appearance in matches is used. In practice this is probably the value of -Ctarget-cpus last appearance in the command line arguments of the rustc invocation.
Will add a test soon, but I am asking myself what is the intended behavior? I assume we should error if a target-modifier appears more than once, would you agree?
Member
There was a problem hiding this comment.
At the very least we have to ensure that whatever values rustc actually uses and what is recorded as target modifier are the same.
Contributor
Author
There was a problem hiding this comment.
Last argument taking precedence is the desired functionality:
Corresponding MCP
Member
There was a problem hiding this comment.
Behavior as per MCP is fine. Whichever value ends up being used should be tracked and verified.
Contributor
Author
There was a problem hiding this comment.
Is checked now by tests/run-make/target-cpu-is-target-modifier/rmake.rs
Does that match what you had in mind?
Darksonn
reviewed
Jan 10, 2026
kulst
force-pushed
the
cpu_is_target_modifier
branch
from
a8f5e96 to
e5cbf49
Compare
Collaborator
|
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr The reflection data structures are tied exactly to the implementation cc @oli-obk
Some changes occurred in tests/ui/stack-protector cc @rust-lang/project-exploit-mitigations, @rcvalle Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred to the CTFE machinery |
rustbot
added
A-CI
rustbot
added
O-AVR
This comment has been minimized.
This comment has been minimized.
kulst
force-pushed
the
cpu_is_target_modifier
branch
from
c9bc79b to
ba01dc2
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kulst
force-pushed
the
cpu_is_target_modifier
branch
from
ba01dc2 to
5cbc2a1
Compare
This comment has been minimized.
This comment has been minimized.
Member
Contributor
|
The idea sounds good to me. Technically, amdgpu could allow linking different target-cpus with generic targets (e.g. |
Contributor
|
Thanks for ping - looks good to me as well! |
Contributor
Author
|
Thank you both for stopping by and approving the changes. |
Member
|
@rustbot author |
rustbot
added
S-waiting-on-author
Collaborator
|
Reminder, once the PR becomes ready for a review, use |
Member
|
ping @kulst since you meant to come back to it last month (No rush if work/study still keep you busy, just in case you forgot about it). |
Contributor
Author
|
After revisiting this PR I'd like to discuss how to move forward with it.
The PR, as is, would change the following:
This would have the following implications:
How I would move forward:
Another option is to wait for the |
Member
|
By "destabilizing" the NVPTX target, you mean moving it to tier 3? We don't have a notion of "target stability", just "target tiers". Cc @kjetilkjeka
As long as this is all nightly-only, that sounds good to me. |
Contributor
Author
By destabilizing I meant removing the ability to build for this target using a stable compiler. The target tier policy states that for Tier 2 targets the Rust project guarantees that a target builds and Tier 2 targets must build reliably in CI, for all components that Rust’s CI considers mandatory. To me that reads the same as it must be possible to build for that target using any compiler released by the Rust project (including stable ones) at a time the target is in Tier 2. Since we would fail that requirement, a demotion of NVPTX to Tier 3 would be necessary. |
3 tasks
- Boolean target modifiers are now mentioned without a trailing `=` in the messages. - Wording improved for unset target modifiers.
kulst
force-pushed
the
cpu_is_target_modifier
branch
from
5cbc2a1 to
b6d3b44
Compare
rustbot
added
the
A-LLVM
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. |
This comment has been minimized.
This comment has been minimized.
…and NVPTX For AVR, AMDGCN, and NVPTX, crates built with different target CPU values are not generally link-compatible. Add a `requires_consistent_cpu` flag to the target spec and enable it for these targets. When the flag is set, treat `-Ctarget-cpu` as a target modifier and require all linked crates to agree on its value. Reject `-Ctarget-cpu=native` before codegen for targets that set `requires_consistent_cpu` to true. Also do not include `native` in the printed `target-cpus` list for such targets. Add tests covering: - which built-in targets set `requires-consistent-cpu` - cross-crate behavior with and without `requires-consistent-cpu` - that an omitted `-Ctarget-cpu` compares equal to an explicitly specified default CPU - rejection and printing behavior for `native` - precedence of repeated `-Ctarget-cpu` flags in metadata comparison and LLVM IR
kulst
force-pushed
the
cpu_is_target_modifier
branch
from
b6d3b44 to
d17453e
Compare
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
View all comments
Crates built for AVR, AMDGCN and NVPTX that specify different values for
-Ctarget-cpucannot be soundly linked together. Thereforeneed_explicit_cpuwas already set in their target specifications (NVPTX has the same issue, but conversely this flag was never set for it).This PR attempts to make
rustcensure that no crates with disagreeing values for-Ctarget-cpuare linked together by it. This is achieved by converting-Ctarget-cpuinto a target-modifier. The agreement is only enforced for the mentioned targets based on the value inneed_explicit_cpuTo express this additional requirement, the flag is renamed intorequires_explicit_and_consistent_cpu.Why should
-Ctarget-cpube a target-modifier fornvptx?PTX is a single-module contract
PTX requires a binary to start with .version (`ptx$$`) then .target (`sm_$$`). If the ptx contains instructions that are not supported by either .version or .target, the binary is ill-formed and will be rejected by ptxas. The concept of features that can be mixed and matched in a binary does not exist for nvptx and is therefore not supported by LLVM.
It prevents the production of bitcode that cannot be codegen'd after linking
A target modifier should prevent configurations that are not composable across crates when those crates are linked together. The most prominent example is when enabling a target feature changes the ABI, making cross-crate calls inherently unsound.
In the case of nvptx, ABI mismatch is (at least for now) not the core problem motivating target modifiers. NVIDIA’s documented PTX calling convention has remained stable since ptx20.
However, in the current state it is possible to produce bitcode that cannot be codegen'd after linking, because some operations are only lowerable for sufficiently new SM/PTX levels. In the best case this results in an LLVM error during the final llc step, but this is not something we should rely on for correctness.
nvptx has a special compilation pipeline where instead of linking the final PTX object, instead LLVM bitcode is linked. The resulting artifact is then compiled in one invocation.
Now consider crate A which is independently compiled into bitcode with the following rustc arguments:
Crate A is a dependency of crate B. In the rustc invocation of crate B
This should now ideally create an LLVM error, because the linked bitcode contains code paths that were selected under
sm_70assumptions but the final NVPTX codegen is targetingsm_60, where those operations are not lowerable. An LLVM error here is better than silent miscompilation, but it’s not a promise we should rely on.A real example where this could happen is the lowering of atomic loads and stores with non-relaxed orderings, which is known to depend on the selected SM level.
Why should
-Ctarget-cpube a target-modifier foramdgcnandavr?Previous discussions about the topic can be found here and here.
I also created a Zulip discussion.
I am unsure if a MCP is needed before proceeding. If you think so please let me know.
Creating target-modifiers for NVPTX target-features is to be done in a follow-up.
cc @kjetilkjeka as target maintainer for NVPTX
cc @Flakebi as target maintainer for amdgcn
cc @Patryk27 as target maintainer for AVR
cc @RalfJung you were very involved in the discussions so far
Target modifier tracking issue: #136966