Target option to require explicit cpu #135030
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Nadrieril (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
|
These commits modify compiler targets. |
This comment has been minimized.
This comment has been minimized.
|
Looks reasonable but I lack context. r? compiler |
| if tcx.sess.target.need_explicit_cpu && tcx.sess.opts.cg.target_cpu.is_none() { | ||
| // The target has no default cpu, but none is set explicitly | ||
| tcx.dcx().emit_fatal(errors::CpuRequired); | ||
| } |
There was a problem hiding this comment.
Maybe this should instead error when creating the Session? If target features depend on -Ctarget-cpu, the current code would allow a check build to be done with the wrong target features available through #[cfg(target_feature)] if -Ctarget-cpu isn't used.
There was a problem hiding this comment.
Hm, I tried moving the code into validate_commandline_args_with_session_available, but that makes basic commands fail that don’t try to compile any code.
$ rustc --print target-cpus --target amdgcn-amd-amdhsa
error: target requires to explicitly specify a cpu with `-C target-cpu`and for cargo
$ cargo check
error: failed to run `rustc` to learn about target-specific information
Caused by:
process didn't exit successfully: `rustc - --crate-name ___ --print=file-names --target amdgcn-amd-amdhsa --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=crate-name --print=cfg` (exit status: 1)
--- stderr
error: target requires to explicitly specify a cpu with `-C target-cpu`(I also tried running cargo check without moving the code and it failed without -Ctarget-cpu.)
There was a problem hiding this comment.
Ah, right. Checking it when building the Session would prevent anything that internally uses the Session from working without -Ctarget-cpu, including those that don't need to know about the target cpu.
Some targets have many different CPUs and no generic CPU that can be used as a default. For these targets, the user needs to explicitly specify a CPU through `-C target-cpu=`. Add an option for targets and an error message if no CPU is set. This affects the proposed amdgpu and avr targets.
|
This PR modifies cc @jieyouxu |
|
|
@bors r+ |
bors
added
S-waiting-on-bors
|
This is blocking PRs which are blocking PRs. @bors p=2 |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (a730edc): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary 0.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 2.1%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 778.592s -> 778.256s (-0.04%) |
Some targets have many different CPUs and no generic CPU that can be used as a default. For these targets, the user needs to explicitly specify a CPU through
-C target-cpu=.Add an option for targets and an error message if no CPU is set.
This affects the proposed amdgpu and avr targets.
amdgpu tracking issue: #135024
AVR MCP: rust-lang/compiler-team#800