Skip to content

Commit 20d4c39

Browse files
committed
Correctly handle target features
1 parent f692124 commit 20d4c39

File tree

5 files changed

+289
-67
lines changed

5 files changed

+289
-67
lines changed

messages.ftl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
codegen_gcc_unknown_ctarget_feature_prefix =
2+
unknown feature specified for `-Ctarget-feature`: `{$feature}`
3+
.note = features must begin with a `+` to enable or `-` to disable it
4+
15
codegen_gcc_invalid_minimum_alignment =
26
invalid minimum global alignment: {$err}
37
@@ -23,3 +27,15 @@ codegen_gcc_lto_disallowed = lto can only be run for executables, cdylibs and st
2327
codegen_gcc_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdylib-lto`
2428
2529
codegen_gcc_lto_bitcode_from_rlib = failed to get bitcode from object file for LTO ({$gcc_err})
30+
31+
codegen_gcc_unknown_ctarget_feature =
32+
unknown feature specified for `-Ctarget-feature`: `{$feature}`
33+
.note = it is still passed through to the codegen backend
34+
.possible_feature = you might have meant: `{$rust_feature}`
35+
.consider_filing_feature_request = consider filing a feature request
36+
37+
codegen_gcc_missing_features =
38+
add the missing features in a `target_feature` attribute
39+
40+
codegen_gcc_target_feature_disable_or_enable =
41+
the target features {$features} must all be either enabled or disabled together

src/attributes.rs

Lines changed: 23 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -4,72 +4,13 @@ use gccjit::Function;
44
use rustc_attr::InstructionSetAttr;
55
#[cfg(feature="master")]
66
use rustc_attr::InlineAttr;
7-
use rustc_codegen_ssa::target_features::tied_target_features;
8-
use rustc_data_structures::fx::FxHashMap;
97
use rustc_middle::ty;
108
#[cfg(feature="master")]
119
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
12-
use rustc_session::Session;
1310
use rustc_span::symbol::sym;
14-
use smallvec::{smallvec, SmallVec};
1511

1612
use crate::{context::CodegenCx, errors::TiedTargetFeatures};
17-
18-
// Given a map from target_features to whether they are enabled or disabled,
19-
// ensure only valid combinations are allowed.
20-
pub fn check_tied_features(sess: &Session, features: &FxHashMap<&str, bool>) -> Option<&'static [&'static str]> {
21-
for tied in tied_target_features(sess) {
22-
// Tied features must be set to the same value, or not set at all
23-
let mut tied_iter = tied.iter();
24-
let enabled = features.get(tied_iter.next().unwrap());
25-
if tied_iter.any(|feature| enabled != features.get(feature)) {
26-
return Some(tied);
27-
}
28-
}
29-
None
30-
}
31-
32-
// TODO(antoyo): maybe move to a new module gcc_util.
33-
// To find a list of GCC's names, check https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
34-
fn to_gcc_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]> {
35-
let arch = if sess.target.arch == "x86_64" { "x86" } else { &*sess.target.arch };
36-
match (arch, s) {
37-
("x86", "sse4.2") => smallvec!["sse4.2", "crc32"],
38-
("x86", "pclmulqdq") => smallvec!["pclmul"],
39-
("x86", "rdrand") => smallvec!["rdrnd"],
40-
("x86", "bmi1") => smallvec!["bmi"],
41-
("x86", "cmpxchg16b") => smallvec!["cx16"],
42-
("x86", "avx512vaes") => smallvec!["vaes"],
43-
("x86", "avx512gfni") => smallvec!["gfni"],
44-
("x86", "avx512vpclmulqdq") => smallvec!["vpclmulqdq"],
45-
// NOTE: seems like GCC requires 'avx512bw' for 'avx512vbmi2'.
46-
("x86", "avx512vbmi2") => smallvec!["avx512vbmi2", "avx512bw"],
47-
// NOTE: seems like GCC requires 'avx512bw' for 'avx512bitalg'.
48-
("x86", "avx512bitalg") => smallvec!["avx512bitalg", "avx512bw"],
49-
("aarch64", "rcpc2") => smallvec!["rcpc-immo"],
50-
("aarch64", "dpb") => smallvec!["ccpp"],
51-
("aarch64", "dpb2") => smallvec!["ccdp"],
52-
("aarch64", "frintts") => smallvec!["fptoint"],
53-
("aarch64", "fcma") => smallvec!["complxnum"],
54-
("aarch64", "pmuv3") => smallvec!["perfmon"],
55-
("aarch64", "paca") => smallvec!["pauth"],
56-
("aarch64", "pacg") => smallvec!["pauth"],
57-
// Rust ties fp and neon together. In LLVM neon implicitly enables fp,
58-
// but we manually enable neon when a feature only implicitly enables fp
59-
("aarch64", "f32mm") => smallvec!["f32mm", "neon"],
60-
("aarch64", "f64mm") => smallvec!["f64mm", "neon"],
61-
("aarch64", "fhm") => smallvec!["fp16fml", "neon"],
62-
("aarch64", "fp16") => smallvec!["fullfp16", "neon"],
63-
("aarch64", "jsconv") => smallvec!["jsconv", "neon"],
64-
("aarch64", "sve") => smallvec!["sve", "neon"],
65-
("aarch64", "sve2") => smallvec!["sve2", "neon"],
66-
("aarch64", "sve2-aes") => smallvec!["sve2-aes", "neon"],
67-
("aarch64", "sve2-sm4") => smallvec!["sve2-sm4", "neon"],
68-
("aarch64", "sve2-sha3") => smallvec!["sve2-sha3", "neon"],
69-
("aarch64", "sve2-bitperm") => smallvec!["sve2-bitperm", "neon"],
70-
(_, s) => smallvec![s],
71-
}
72-
}
13+
use crate::gcc_util::{check_tied_features, to_gcc_features};
7314

7415
/// Get GCC attribute for the provided inline heuristic.
7516
#[cfg(feature="master")]
@@ -153,12 +94,31 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
15394
}))
15495
.collect::<Vec<_>>();
15596

156-
// TODO(antoyo): check if we really need global backend features. (Maybe they could be applied
157-
// globally?)
97+
// TODO(antoyo): cg_llvm add global features to each function so that LTO keep them.
98+
// Check if GCC requires the same.
15899
let mut global_features = cx.tcx.global_backend_features(()).iter().map(|s| s.as_str());
159100
function_features.extend(&mut global_features);
160-
let target_features = function_features.join(",");
101+
let target_features = function_features
102+
.iter()
103+
.filter_map(|feature| {
104+
if feature.contains("soft-float") || feature.contains("retpoline-external-thunk") {
105+
return None;
106+
}
107+
108+
if feature.starts_with('-') {
109+
Some(format!("no{}", feature))
110+
}
111+
else if feature.starts_with('+') {
112+
Some(feature[1..].to_string())
113+
}
114+
else {
115+
Some(feature.to_string())
116+
}
117+
})
118+
.collect::<Vec<_>>()
119+
.join(",");
161120
if !target_features.is_empty() {
121+
println!("Function {:?}", function_features);
162122
#[cfg(feature="master")]
163123
func.add_attribute(FnAttribute::Target(&target_features));
164124
}

src/errors.rs

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,36 @@
1-
use rustc_errors::{DiagnosticArgValue, IntoDiagnosticArg};
2-
use rustc_macros::Diagnostic;
1+
use rustc_errors::{
2+
DiagnosticArgValue, DiagnosticBuilder, ErrorGuaranteed, Handler, IntoDiagnostic, IntoDiagnosticArg,
3+
};
4+
use rustc_macros::{Diagnostic, Subdiagnostic};
35
use rustc_span::Span;
46
use std::borrow::Cow;
57

8+
use crate::fluent_generated as fluent;
9+
10+
#[derive(Diagnostic)]
11+
#[diag(codegen_gcc_unknown_ctarget_feature_prefix)]
12+
#[note]
13+
pub(crate) struct UnknownCTargetFeaturePrefix<'a> {
14+
pub feature: &'a str,
15+
}
16+
17+
#[derive(Diagnostic)]
18+
#[diag(codegen_gcc_unknown_ctarget_feature)]
19+
#[note]
20+
pub(crate) struct UnknownCTargetFeature<'a> {
21+
pub feature: &'a str,
22+
#[subdiagnostic]
23+
pub rust_feature: PossibleFeature<'a>,
24+
}
25+
26+
#[derive(Subdiagnostic)]
27+
pub(crate) enum PossibleFeature<'a> {
28+
#[help(codegen_gcc_possible_feature)]
29+
Some { rust_feature: &'a str },
30+
#[help(codegen_gcc_consider_filing_feature_request)]
31+
None,
32+
}
33+
634
struct ExitCode(Option<i32>);
735

836
impl IntoDiagnosticArg for ExitCode {
@@ -71,3 +99,27 @@ pub(crate) struct LtoDylib;
7199
pub(crate) struct LtoBitcodeFromRlib {
72100
pub gcc_err: String,
73101
}
102+
103+
pub(crate) struct TargetFeatureDisableOrEnable<'a> {
104+
pub features: &'a [&'a str],
105+
pub span: Option<Span>,
106+
pub missing_features: Option<MissingFeatures>,
107+
}
108+
109+
#[derive(Subdiagnostic)]
110+
#[help(codegen_gcc_missing_features)]
111+
pub(crate) struct MissingFeatures;
112+
113+
impl IntoDiagnostic<'_, ErrorGuaranteed> for TargetFeatureDisableOrEnable<'_> {
114+
fn into_diagnostic(self, sess: &'_ Handler) -> DiagnosticBuilder<'_, ErrorGuaranteed> {
115+
let mut diag = sess.struct_err(fluent::codegen_gcc_target_feature_disable_or_enable);
116+
if let Some(span) = self.span {
117+
diag.set_span(span);
118+
};
119+
if let Some(missing_features) = self.missing_features {
120+
diag.subdiagnostic(missing_features);
121+
}
122+
diag.set_arg("features", self.features.join(", "));
123+
diag
124+
}
125+
}

src/gcc_util.rs

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
use smallvec::{smallvec, SmallVec};
2+
3+
use rustc_codegen_ssa::target_features::{
4+
supported_target_features, tied_target_features, RUSTC_SPECIFIC_FEATURES,
5+
};
6+
use rustc_data_structures::fx::FxHashMap;
7+
use rustc_middle::bug;
8+
use rustc_session::Session;
9+
10+
use crate::errors::{PossibleFeature, TargetFeatureDisableOrEnable, UnknownCTargetFeature, UnknownCTargetFeaturePrefix};
11+
12+
/// The list of GCC features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`,
13+
/// `--target` and similar).
14+
pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<String> {
15+
// Features that come earlier are overridden by conflicting features later in the string.
16+
// Typically we'll want more explicit settings to override the implicit ones, so:
17+
//
18+
// * Features from -Ctarget-cpu=*; are overridden by [^1]
19+
// * Features implied by --target; are overridden by
20+
// * Features from -Ctarget-feature; are overridden by
21+
// * function specific features.
22+
//
23+
// [^1]: target-cpu=native is handled here, other target-cpu values are handled implicitly
24+
// through GCC TargetMachine implementation.
25+
//
26+
// FIXME(nagisa): it isn't clear what's the best interaction between features implied by
27+
// `-Ctarget-cpu` and `--target` are. On one hand, you'd expect CLI arguments to always
28+
// override anything that's implicit, so e.g. when there's no `--target` flag, features implied
29+
// the host target are overridden by `-Ctarget-cpu=*`. On the other hand, what about when both
30+
// `--target` and `-Ctarget-cpu=*` are specified? Both then imply some target features and both
31+
// flags are specified by the user on the CLI. It isn't as clear-cut which order of precedence
32+
// should be taken in cases like these.
33+
let mut features = vec![];
34+
35+
// TODO(antoyo): -Ctarget-cpu=native
36+
37+
// Features implied by an implicit or explicit `--target`.
38+
features.extend(
39+
sess.target
40+
.features
41+
.split(',')
42+
.filter(|v| !v.is_empty() && backend_feature_name(v).is_some())
43+
.map(String::from),
44+
);
45+
46+
// -Ctarget-features
47+
let supported_features = supported_target_features(sess);
48+
let mut featsmap = FxHashMap::default();
49+
let feats = sess.opts.cg.target_feature
50+
.split(',')
51+
.filter_map(|s| {
52+
let enable_disable = match s.chars().next() {
53+
None => return None,
54+
Some(c @ ('+' | '-')) => c,
55+
Some(_) => {
56+
if diagnostics {
57+
sess.emit_warning(UnknownCTargetFeaturePrefix { feature: s });
58+
}
59+
return None;
60+
}
61+
};
62+
63+
let feature = backend_feature_name(s)?;
64+
// Warn against use of GCC specific feature names on the CLI.
65+
if diagnostics && !supported_features.iter().any(|&(v, _)| v == feature) {
66+
let rust_feature = supported_features.iter().find_map(|&(rust_feature, _)| {
67+
let gcc_features = to_gcc_features(sess, rust_feature);
68+
if gcc_features.contains(&feature) && !gcc_features.contains(&rust_feature) {
69+
Some(rust_feature)
70+
} else {
71+
None
72+
}
73+
});
74+
let unknown_feature =
75+
if let Some(rust_feature) = rust_feature {
76+
UnknownCTargetFeature {
77+
feature,
78+
rust_feature: PossibleFeature::Some { rust_feature },
79+
}
80+
}
81+
else {
82+
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
83+
};
84+
sess.emit_warning(unknown_feature);
85+
}
86+
87+
if diagnostics {
88+
// FIXME(nagisa): figure out how to not allocate a full hashset here.
89+
featsmap.insert(feature, enable_disable == '+');
90+
}
91+
92+
// rustc-specific features do not get passed down to GCC…
93+
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
94+
return None;
95+
}
96+
// ... otherwise though we run through `to_gcc_features` when
97+
// passing requests down to GCC. This means that all in-language
98+
// features also work on the command line instead of having two
99+
// different names when the GCC name and the Rust name differ.
100+
Some(to_gcc_features(sess, feature)
101+
.iter()
102+
.flat_map(|feat| to_gcc_features(sess, feat).into_iter())
103+
.map(String::from)
104+
.collect::<Vec<_>>(),
105+
)
106+
})
107+
.flatten();
108+
features.extend(feats);
109+
110+
if diagnostics {
111+
if let Some(f) = check_tied_features(sess, &featsmap) {
112+
sess.emit_err(TargetFeatureDisableOrEnable {
113+
features: f,
114+
span: None,
115+
missing_features: None,
116+
});
117+
}
118+
}
119+
120+
features
121+
}
122+
123+
/// Returns a feature name for the given `+feature` or `-feature` string.
124+
///
125+
/// Only allows features that are backend specific (i.e. not [`RUSTC_SPECIFIC_FEATURES`].)
126+
fn backend_feature_name(s: &str) -> Option<&str> {
127+
// features must start with a `+` or `-`.
128+
let feature = s.strip_prefix(&['+', '-'][..]).unwrap_or_else(|| {
129+
bug!("target feature `{}` must begin with a `+` or `-`", s);
130+
});
131+
// Rustc-specific feature requests like `+crt-static` or `-crt-static`
132+
// are not passed down to GCC.
133+
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
134+
return None;
135+
}
136+
Some(feature)
137+
}
138+
139+
// TODO(antoyo): maybe move to a new module gcc_util.
140+
// To find a list of GCC's names, check https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
141+
pub fn to_gcc_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]> {
142+
let arch = if sess.target.arch == "x86_64" { "x86" } else { &*sess.target.arch };
143+
match (arch, s) {
144+
("x86", "sse4.2") => smallvec!["sse4.2", "crc32"],
145+
("x86", "pclmulqdq") => smallvec!["pclmul"],
146+
("x86", "rdrand") => smallvec!["rdrnd"],
147+
("x86", "bmi1") => smallvec!["bmi"],
148+
("x86", "cmpxchg16b") => smallvec!["cx16"],
149+
("x86", "avx512vaes") => smallvec!["vaes"],
150+
("x86", "avx512gfni") => smallvec!["gfni"],
151+
("x86", "avx512vpclmulqdq") => smallvec!["vpclmulqdq"],
152+
// NOTE: seems like GCC requires 'avx512bw' for 'avx512vbmi2'.
153+
("x86", "avx512vbmi2") => smallvec!["avx512vbmi2", "avx512bw"],
154+
// NOTE: seems like GCC requires 'avx512bw' for 'avx512bitalg'.
155+
("x86", "avx512bitalg") => smallvec!["avx512bitalg", "avx512bw"],
156+
("aarch64", "rcpc2") => smallvec!["rcpc-immo"],
157+
("aarch64", "dpb") => smallvec!["ccpp"],
158+
("aarch64", "dpb2") => smallvec!["ccdp"],
159+
("aarch64", "frintts") => smallvec!["fptoint"],
160+
("aarch64", "fcma") => smallvec!["complxnum"],
161+
("aarch64", "pmuv3") => smallvec!["perfmon"],
162+
("aarch64", "paca") => smallvec!["pauth"],
163+
("aarch64", "pacg") => smallvec!["pauth"],
164+
// Rust ties fp and neon together. In GCC neon implicitly enables fp,
165+
// but we manually enable neon when a feature only implicitly enables fp
166+
("aarch64", "f32mm") => smallvec!["f32mm", "neon"],
167+
("aarch64", "f64mm") => smallvec!["f64mm", "neon"],
168+
("aarch64", "fhm") => smallvec!["fp16fml", "neon"],
169+
("aarch64", "fp16") => smallvec!["fullfp16", "neon"],
170+
("aarch64", "jsconv") => smallvec!["jsconv", "neon"],
171+
("aarch64", "sve") => smallvec!["sve", "neon"],
172+
("aarch64", "sve2") => smallvec!["sve2", "neon"],
173+
("aarch64", "sve2-aes") => smallvec!["sve2-aes", "neon"],
174+
("aarch64", "sve2-sm4") => smallvec!["sve2-sm4", "neon"],
175+
("aarch64", "sve2-sha3") => smallvec!["sve2-sha3", "neon"],
176+
("aarch64", "sve2-bitperm") => smallvec!["sve2-bitperm", "neon"],
177+
(_, s) => smallvec![s],
178+
}
179+
}
180+
181+
// Given a map from target_features to whether they are enabled or disabled,
182+
// ensure only valid combinations are allowed.
183+
pub fn check_tied_features(sess: &Session, features: &FxHashMap<&str, bool>) -> Option<&'static [&'static str]> {
184+
for tied in tied_target_features(sess) {
185+
// Tied features must be set to the same value, or not set at all
186+
let mut tied_iter = tied.iter();
187+
let enabled = features.get(tied_iter.next().unwrap());
188+
if tied_iter.any(|feature| enabled != features.get(feature)) {
189+
return Some(tied);
190+
}
191+
}
192+
None
193+
}

0 commit comments

Comments
 (0)