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

rustc_target: add known safe s390x target features #127506

Merged
merged 4 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ fn stackprotector_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attribute> {
Some(sspattr.create_attr(cx.llcx))
}

fn backchain_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attribute> {
if cx.sess().target.arch != "s390x" {
return None;
}

let requested_features = cx.sess().opts.cg.target_feature.split(',');
let found_positive = requested_features.clone().any(|r| r == "+backchain");

if found_positive { Some(llvm::CreateAttrString(cx.llcx, "backchain")) } else { None }
}

pub fn target_cpu_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> &'ll Attribute {
let target_cpu = llvm_util::target_cpu(cx.tcx.sess);
llvm::CreateAttrStringValue(cx.llcx, "target-cpu", target_cpu)
Expand Down Expand Up @@ -447,6 +458,9 @@ pub fn from_fn_attrs<'ll, 'tcx>(
if let Some(align) = codegen_fn_attrs.alignment {
llvm::set_alignment(llfn, align);
}
if let Some(backchain) = backchain_attr(cx) {
to_add.push(backchain);
}
to_add.extend(sanitize_attrs(cx, codegen_fn_attrs.no_sanitize));
to_add.extend(patchable_function_entry_attrs(cx, codegen_fn_attrs.patchable_function_entry));

Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_session::config::{PrintKind, PrintRequest};
use rustc_session::Session;
use rustc_span::symbol::Symbol;
use rustc_target::spec::{MergeFunctions, PanicStrategy};
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES};

use std::ffi::{c_char, c_void, CStr, CString};
use std::fmt::Write;
Expand Down Expand Up @@ -321,6 +321,10 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
}
})
.filter(|feature| {
// skip checking special features, as LLVM may not understands them
if RUSTC_SPECIAL_FEATURES.contains(feature) {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

This makes it so that cfg!(target_feature = "backchain") will always be true, even if it is not enabled. Is that the intended behavior? It seems very odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is to skip checking if LLVM supports this attribute. The logic for adding this attribute is somewhere else.
If you are unconvinced, check out this Godbolt playground: https://godbolt.org/z/77hrrsbo1.

Copy link
Member

Choose a reason for hiding this comment

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

I am convinced that the logic is wrong, see #129927.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I have no idea what I am supposed to look at in this Godbolt.^^ I know nothing about s390x or backchain, I am just noticing odd things in the rustc source code.

// check that all features in a given smallvec are enabled
for llvm_feature in to_llvm_features(sess, feature) {
let cstr = SmallCStr::new(llvm_feature);
Expand Down Expand Up @@ -546,6 +550,7 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str

// -Ctarget-features
let supported_features = sess.target.supported_target_features();
let (llvm_major, _, _) = get_version();
let mut featsmap = FxHashMap::default();
let feats = sess
.opts
Expand Down Expand Up @@ -604,6 +609,13 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
return None;
}

// if the target-feature is "backchain" and LLVM version is greater than 18
// then we also need to add "+backchain" to the target-features attribute.
// otherwise, we will only add the naked `backchain` attribute to the attribute-group.
if feature == "backchain" && llvm_major < 18 {
Copy link
Member

Choose a reason for hiding this comment

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

This entire RUSTC_SPECIAL_FEATURES is a temporary hack we can drop when we start requiring LLVM 18, right? Please file an issue to track this cleanup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This entire RUSTC_SPECIAL_FEATURES is a temporary hack we can drop when we start requiring LLVM 18, right? Please file an issue to track this cleanup.

Yes, this is due to older LLVM does not support adding this attribute to the target-features attribute.

return None;
}
// ... otherwise though we run through `to_llvm_features` when
// passing requests down to LLVM. This means that all in-language
// features also work on the command line instead of having two
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ pub fn from_target_feature(
Some(sym::prfchw_target_feature) => rust_features.prfchw_target_feature,
Some(sym::x86_amx_intrinsics) => rust_features.x86_amx_intrinsics,
Some(sym::xop_target_feature) => rust_features.xop_target_feature,
Some(sym::s390x_target_feature) => rust_features.s390x_target_feature,
Some(name) => bug!("unknown target feature gate {}", name),
None => true,
};
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ declare_features! (
(unstable, prfchw_target_feature, "1.78.0", Some(44839)),
(unstable, riscv_target_feature, "1.45.0", Some(44839)),
(unstable, rtm_target_feature, "1.35.0", Some(44839)),
(unstable, s390x_target_feature, "CURRENT_RUSTC_VERSION", Some(44839)),
(unstable, sse4a_target_feature, "1.27.0", Some(44839)),
(unstable, tbm_target_feature, "1.27.0", Some(44839)),
(unstable, wasm_target_feature, "1.30.0", Some(44839)),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1689,6 +1689,7 @@ symbols! {
rvalue_static_promotion,
rwpi,
s,
s390x_target_feature,
safety,
sanitize,
sanitizer_cfi_generalize_pointers,
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_target/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ use rustc_span::symbol::Symbol;
/// Features that control behaviour of rustc, rather than the codegen.
pub const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"];

/// Features that require special handling when passing to LLVM.
pub const RUSTC_SPECIAL_FEATURES: &[&str] = &["backchain"];

/// Stability information for target features.
#[derive(Debug, Clone, Copy)]
pub enum Stability {
Expand Down Expand Up @@ -397,6 +400,13 @@ const LOONGARCH_ALLOWED_FEATURES: &[(&str, Stability)] = &[
// tidy-alphabetical-end
];

const IBMZ_ALLOWED_FEATURES: &[(&str, Stability)] = &[
// tidy-alphabetical-start
("backchain", Unstable(sym::s390x_target_feature)),
("vector", Unstable(sym::s390x_target_feature)),
// tidy-alphabetical-end
];

/// When rustdoc is running, provide a list of all known features so that all their respective
/// primitives may be documented.
///
Expand All @@ -414,6 +424,7 @@ pub fn all_known_features() -> impl Iterator<Item = (&'static str, Stability)> {
.chain(BPF_ALLOWED_FEATURES.iter())
.chain(CSKY_ALLOWED_FEATURES)
.chain(LOONGARCH_ALLOWED_FEATURES)
.chain(IBMZ_ALLOWED_FEATURES)
.cloned()
}

Expand All @@ -431,6 +442,7 @@ impl super::spec::Target {
"bpf" => BPF_ALLOWED_FEATURES,
"csky" => CSKY_ALLOWED_FEATURES,
"loongarch64" => LOONGARCH_ALLOWED_FEATURES,
"s390x" => IBMZ_ALLOWED_FEATURES,
_ => &[],
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/mix.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ warning: unexpected `cfg` condition value: `zebra`
LL | cfg!(target_feature = "zebra");
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: expected values for `target_feature` are: `10e60`, `2e3`, `3e3r1`, `3e3r2`, `3e3r3`, `3e7`, `7e10`, `a`, `aclass`, `adx`, `aes`, `altivec`, `alu32`, `amx-bf16`, `amx-complex`, `amx-fp16`, `amx-int8`, `amx-tile`, `atomics`, `avx`, `avx2`, `avx512bf16`, `avx512bitalg`, `avx512bw`, `avx512cd`, `avx512dq`, `avx512f`, `avx512fp16`, `avx512ifma`, `avx512vbmi`, `avx512vbmi2`, `avx512vl`, `avx512vnni`, `avx512vp2intersect`, and `avx512vpopcntdq` and 197 more
= note: expected values for `target_feature` are: `10e60`, `2e3`, `3e3r1`, `3e3r2`, `3e3r3`, `3e7`, `7e10`, `a`, `aclass`, `adx`, `aes`, `altivec`, `alu32`, `amx-bf16`, `amx-complex`, `amx-fp16`, `amx-int8`, `amx-tile`, `atomics`, `avx`, `avx2`, `avx512bf16`, `avx512bitalg`, `avx512bw`, `avx512cd`, `avx512dq`, `avx512f`, `avx512fp16`, `avx512ifma`, `avx512vbmi`, `avx512vbmi2`, `avx512vl`, `avx512vnni`, `avx512vp2intersect`, and `avx512vpopcntdq` and 199 more
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration

warning: 27 warnings emitted
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/well-known-values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE`
LL | target_feature = "_UNEXPECTED_VALUE",
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: expected values for `target_feature` are: `10e60`, `2e3`, `3e3r1`, `3e3r2`, `3e3r3`, `3e7`, `7e10`, `a`, `aclass`, `adx`, `aes`, `altivec`, `alu32`, `amx-bf16`, `amx-complex`, `amx-fp16`, `amx-int8`, `amx-tile`, `atomics`, `avx`, `avx2`, `avx512bf16`, `avx512bitalg`, `avx512bw`, `avx512cd`, `avx512dq`, `avx512f`, `avx512fp16`, `avx512ifma`, `avx512vbmi`, `avx512vbmi2`, `avx512vl`, `avx512vnni`, `avx512vp2intersect`, `avx512vpopcntdq`, `avxifma`, `avxneconvert`, `avxvnni`, `avxvnniint16`, `avxvnniint8`, `bf16`, `bmi1`, `bmi2`, `bti`, `bulk-memory`, `c`, `cache`, `cmpxchg16b`, `crc`, `crt-static`, `d`, `d32`, `dit`, `doloop`, `dotprod`, `dpb`, `dpb2`, `dsp`, `dsp1e2`, `dspe60`, `e`, `e1`, `e2`, `edsp`, `elrw`, `ermsb`, `exception-handling`, `extended-const`, `f`, `f16c`, `f32mm`, `f64mm`, `fcma`, `fdivdu`, `fhm`, `flagm`, `float1e2`, `float1e3`, `float3e4`, `float7e60`, `floate1`, `fma`, `fp-armv8`, `fp16`, `fp64`, `fpuv2_df`, `fpuv2_sf`, `fpuv3_df`, `fpuv3_hf`, `fpuv3_hi`, `fpuv3_sf`, `frecipe`, `frintts`, `fxsr`, `gfni`, `hard-float`, `hard-float-abi`, `hard-tp`, `high-registers`, `hvx`, `hvx-length128b`, `hwdiv`, `i8mm`, `jsconv`, `lahfsahf`, `lasx`, `lbt`, `lor`, `lse`, `lsx`, `lvz`, `lzcnt`, `m`, `mclass`, `movbe`, `mp`, `mp1e2`, `msa`, `mte`, `multivalue`, `mutable-globals`, `neon`, `nontrapping-fptoint`, `nvic`, `paca`, `pacg`, `pan`, `pclmulqdq`, `pmuv3`, `popcnt`, `power10-vector`, `power8-altivec`, `power8-vector`, `power9-altivec`, `power9-vector`, `prfchw`, `rand`, `ras`, `rclass`, `rcpc`, `rcpc2`, `rdm`, `rdrand`, `rdseed`, `reference-types`, `relax`, `relaxed-simd`, `rtm`, `sb`, `sha`, `sha2`, `sha3`, `sign-ext`, `simd128`, `sm4`, `spe`, `ssbs`, `sse`, `sse2`, `sse3`, `sse4.1`, `sse4.2`, `sse4a`, `ssse3`, `sve`, `sve2`, `sve2-aes`, `sve2-bitperm`, `sve2-sha3`, `sve2-sm4`, `tbm`, `thumb-mode`, `thumb2`, `tme`, `trust`, `trustzone`, `ual`, `unaligned-scalar-mem`, `v`, `v5te`, `v6`, `v6k`, `v6t2`, `v7`, `v8`, `v8.1a`, `v8.2a`, `v8.3a`, `v8.4a`, `v8.5a`, `v8.6a`, `v8.7a`, `vaes`, `vdsp2e60f`, `vdspv1`, `vdspv2`, `vfp2`, `vfp3`, `vfp4`, `vh`, `virt`, `virtualization`, `vpclmulqdq`, `vsx`, `xop`, `xsave`, `xsavec`, `xsaveopt`, `xsaves`, `zba`, `zbb`, `zbc`, `zbkb`, `zbkc`, `zbkx`, `zbs`, `zdinx`, `zfh`, `zfhmin`, `zfinx`, `zhinx`, `zhinxmin`, `zk`, `zkn`, `zknd`, `zkne`, `zknh`, `zkr`, `zks`, `zksed`, `zksh`, and `zkt`
= note: expected values for `target_feature` are: `10e60`, `2e3`, `3e3r1`, `3e3r2`, `3e3r3`, `3e7`, `7e10`, `a`, `aclass`, `adx`, `aes`, `altivec`, `alu32`, `amx-bf16`, `amx-complex`, `amx-fp16`, `amx-int8`, `amx-tile`, `atomics`, `avx`, `avx2`, `avx512bf16`, `avx512bitalg`, `avx512bw`, `avx512cd`, `avx512dq`, `avx512f`, `avx512fp16`, `avx512ifma`, `avx512vbmi`, `avx512vbmi2`, `avx512vl`, `avx512vnni`, `avx512vp2intersect`, `avx512vpopcntdq`, `avxifma`, `avxneconvert`, `avxvnni`, `avxvnniint16`, `avxvnniint8`, `backchain`, `bf16`, `bmi1`, `bmi2`, `bti`, `bulk-memory`, `c`, `cache`, `cmpxchg16b`, `crc`, `crt-static`, `d`, `d32`, `dit`, `doloop`, `dotprod`, `dpb`, `dpb2`, `dsp`, `dsp1e2`, `dspe60`, `e`, `e1`, `e2`, `edsp`, `elrw`, `ermsb`, `exception-handling`, `extended-const`, `f`, `f16c`, `f32mm`, `f64mm`, `fcma`, `fdivdu`, `fhm`, `flagm`, `float1e2`, `float1e3`, `float3e4`, `float7e60`, `floate1`, `fma`, `fp-armv8`, `fp16`, `fp64`, `fpuv2_df`, `fpuv2_sf`, `fpuv3_df`, `fpuv3_hf`, `fpuv3_hi`, `fpuv3_sf`, `frecipe`, `frintts`, `fxsr`, `gfni`, `hard-float`, `hard-float-abi`, `hard-tp`, `high-registers`, `hvx`, `hvx-length128b`, `hwdiv`, `i8mm`, `jsconv`, `lahfsahf`, `lasx`, `lbt`, `lor`, `lse`, `lsx`, `lvz`, `lzcnt`, `m`, `mclass`, `movbe`, `mp`, `mp1e2`, `msa`, `mte`, `multivalue`, `mutable-globals`, `neon`, `nontrapping-fptoint`, `nvic`, `paca`, `pacg`, `pan`, `pclmulqdq`, `pmuv3`, `popcnt`, `power10-vector`, `power8-altivec`, `power8-vector`, `power9-altivec`, `power9-vector`, `prfchw`, `rand`, `ras`, `rclass`, `rcpc`, `rcpc2`, `rdm`, `rdrand`, `rdseed`, `reference-types`, `relax`, `relaxed-simd`, `rtm`, `sb`, `sha`, `sha2`, `sha3`, `sign-ext`, `simd128`, `sm4`, `spe`, `ssbs`, `sse`, `sse2`, `sse3`, `sse4.1`, `sse4.2`, `sse4a`, `ssse3`, `sve`, `sve2`, `sve2-aes`, `sve2-bitperm`, `sve2-sha3`, `sve2-sm4`, `tbm`, `thumb-mode`, `thumb2`, `tme`, `trust`, `trustzone`, `ual`, `unaligned-scalar-mem`, `v`, `v5te`, `v6`, `v6k`, `v6t2`, `v7`, `v8`, `v8.1a`, `v8.2a`, `v8.3a`, `v8.4a`, `v8.5a`, `v8.6a`, `v8.7a`, `vaes`, `vdsp2e60f`, `vdspv1`, `vdspv2`, `vector`, `vfp2`, `vfp3`, `vfp4`, `vh`, `virt`, `virtualization`, `vpclmulqdq`, `vsx`, `xop`, `xsave`, `xsavec`, `xsaveopt`, `xsaves`, `zba`, `zbb`, `zbc`, `zbkb`, `zbkc`, `zbkx`, `zbs`, `zdinx`, `zfh`, `zfhmin`, `zfinx`, `zhinx`, `zhinxmin`, `zk`, `zkn`, `zknd`, `zkne`, `zknh`, `zkr`, `zks`, `zksed`, `zksh`, and `zkt`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration

warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE`
Expand Down
1 change: 1 addition & 0 deletions tests/ui/target-feature/gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
// gate-test-loongarch_target_feature
// gate-test-lahfsahf_target_feature
// gate-test-prfchw_target_feature
// gate-test-s390x_target_feature

#[target_feature(enable = "avx512bw")]
//~^ ERROR: currently unstable
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/target-feature/gate.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0658]: the target feature `avx512bw` is currently unstable
--> $DIR/gate.rs:25:18
--> $DIR/gate.rs:26:18
|
LL | #[target_feature(enable = "avx512bw")]
| ^^^^^^^^^^^^^^^^^^^
Expand Down
Loading