Skip to content

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Dec 13, 2023

This PR adds all the known target_feature from rustc_codegen_ssa rustc_target to the well known list of check-cfg.

It does so by moving the list from rustc_codegen_ssa to rustc_target rustc_session (I not sure about this, but some of the moved function take a Session), then using it the fill_well_known function.

This already proved to be useful since portable-simd had a bad cfg.

cc @nnethercote (since we discussed it in #118494)

@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2023

r? @TaKO8Ki

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 13, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2023

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the check-cfg-target-features branch from 7ddc854 to 3f58f62 Compare December 13, 2023 09:25
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2023

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the check-cfg-target-features branch from 3f58f62 to ada0e07 Compare December 13, 2023 09:32
Copy link
Member

@GuillaumeGomez GuillaumeGomez left a comment

Choose a reason for hiding this comment

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

Looks super cool, thanks!

@workingjubilee
Copy link
Member

Is there any chance the target features can be somewhere like rustc_target?

@Urgau
Copy link
Member Author

Urgau commented Dec 13, 2023

Is there any chance the target features can be somewhere like rustc_target?

It should be possible if we are willing to publicly expose the individual lists or remove the Session parameter and use a &str instead.

Basically everywhere we can have a dependency on rustc_span should be possible implementation-wise, since it's quite possible that we will need to make the target features pre-interned symbols for perf reason.

Comment on lines 405 to 427
pub fn supported_target_features(sess: &crate::Session) -> &'static [(&'static str, Stability)] {
match &*sess.target.arch {
"arm" => ARM_ALLOWED_FEATURES,
"aarch64" => AARCH64_ALLOWED_FEATURES,
"x86" | "x86_64" => X86_ALLOWED_FEATURES,
"hexagon" => HEXAGON_ALLOWED_FEATURES,
"mips" | "mips32r6" | "mips64" | "mips64r6" => MIPS_ALLOWED_FEATURES,
"powerpc" | "powerpc64" => POWERPC_ALLOWED_FEATURES,
"riscv32" | "riscv64" => RISCV_ALLOWED_FEATURES,
"wasm32" | "wasm64" => WASM_ALLOWED_FEATURES,
"bpf" => BPF_ALLOWED_FEATURES,
"csky" => CSKY_ALLOWED_FEATURES,
"loongarch64" => LOONGARCH_ALLOWED_FEATURES,
_ => &[],
}
}

pub fn tied_target_features(sess: &crate::Session) -> &'static [&'static [&'static str]] {
match &*sess.target.arch {
"aarch64" => AARCH64_TIED_FEATURES,
_ => &[],
}
}
Copy link
Member

Choose a reason for hiding this comment

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

These taking a Session is not particularly intrinsic to their functionality. What matters is that we're burning a path to sess.target.arch, which means we're already going through rustc_target::Target. These can be turned into functions implemented on Target.

Copy link
Member Author

@Urgau Urgau Dec 13, 2023

Choose a reason for hiding this comment

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

Done. This is much cleaner now. Thank you very much.

@Urgau Urgau force-pushed the check-cfg-target-features branch from ada0e07 to fb9b31c Compare December 13, 2023 10:48
Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

With that, the portable-simd change looks good to me. Thanks!

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Dec 14, 2023

Thanks! @bors r+

@bors
Copy link
Collaborator

bors commented Dec 14, 2023

📌 Commit fb9b31c has been approved by TaKO8Ki

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2023
@bors
Copy link
Collaborator

bors commented Dec 14, 2023

⌛ Testing commit fb9b31c with merge b1dc6c0...

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2023
…aKO8Ki

Add all known `target_feature` configs to check-cfg

This PR adds all the known `target_feature` from ~~`rustc_codegen_ssa`~~ `rustc_target` to the well known list of check-cfg.

It does so by moving the list from `rustc_codegen_ssa` to `rustc_target` ~~`rustc_session` (I not sure about this, but some of the moved function take a `Session`)~~, then using it the `fill_well_known` function.

This already proved to be useful since portable-simd had a bad cfg.

cc `@nnethercote` (since we discussed it in rust-lang#118494)
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Dec 14, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 14, 2023
@Urgau Urgau force-pushed the check-cfg-target-features branch from fb9b31c to 3ea1331 Compare December 14, 2023 13:52
@Urgau
Copy link
Member Author

Urgau commented Dec 14, 2023

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

Fixed the conflicts with #118213. Should be ready for re-approval.

@rustbot ready

@workingjubilee
Copy link
Member

@bors r=TaKO8Ki,GuillaumeGomez,workingjubilee

@bors
Copy link
Collaborator

bors commented Dec 15, 2023

📌 Commit 3ea1331 has been approved by TaKO8Ki,GuillaumeGomez,workingjubilee

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2023
…kingjubilee

Rollup of 4 pull requests

Successful merges:

 - rust-lang#118908 (Add all known `target_feature` configs to check-cfg)
 - rust-lang#118933 (Cleanup errors handlers even more)
 - rust-lang#118943 (update `measureme` to 10.1.2 to deduplicate `parking_lot`)
 - rust-lang#118948 (Use the `Waker::noop` API in tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 576a74b into rust-lang:master Dec 15, 2023
@rustbot rustbot added this to the 1.76.0 milestone Dec 15, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2023
Rollup merge of rust-lang#118908 - Urgau:check-cfg-target-features, r=TaKO8Ki,GuillaumeGomez,workingjubilee

Add all known `target_feature` configs to check-cfg

This PR adds all the known `target_feature` from ~~`rustc_codegen_ssa`~~ `rustc_target` to the well known list of check-cfg.

It does so by moving the list from `rustc_codegen_ssa` to `rustc_target` ~~`rustc_session` (I not sure about this, but some of the moved function take a `Session`)~~, then using it the `fill_well_known` function.

This already proved to be useful since portable-simd had a bad cfg.

cc `@nnethercote` (since we discussed it in rust-lang#118494)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants