Skip to content

Commit

Permalink
Don't use LLVM to compute -Ctarget-feature
Browse files Browse the repository at this point in the history
  • Loading branch information
@calebzulawski
calebzulawski committed Aug 7, 2024
1 parent 484aca8 commit a25da07
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 112 deletions.
5 changes: 2 additions & 3 deletions compiler/rustc_codegen_llvm/src/back/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,12 @@ pub fn write_output_file<'ll>(

pub fn create_informational_target_machine(
sess: &Session,
extra_features: bool,
only_base_features: bool,
) -> OwnedTargetMachine {
let config = TargetMachineFactoryConfig { split_dwarf_file: None, output_obj_file: None };
// Can't use query system here quite yet because this function is invoked before the query
// system/tcx is set up.
let features =
if extra_features { llvm_util::global_llvm_features(sess, false) } else { Vec::new() };
let features = llvm_util::global_llvm_features(sess, false, only_base_features);
target_machine_factory(sess, config::OptLevel::No, &features)(config)
.unwrap_or_else(|err| llvm_err(sess.dcx(), err).raise())
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub unsafe fn create_module<'ll>(

// Ensure the data-layout values hardcoded remain the defaults.
{
let tm = crate::back::write::create_informational_target_machine(tcx.sess, true);
let tm = crate::back::write::create_informational_target_machine(tcx.sess, false);
unsafe {
llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, &tm);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl CodegenBackend for LlvmCodegenBackend {

fn provide(&self, providers: &mut Providers) {
providers.global_backend_features =
|tcx, ()| llvm_util::global_llvm_features(tcx.sess, true)
|tcx, ()| llvm_util::global_llvm_features(tcx.sess, true, false)
}

fn print(&self, req: &PrintRequest, out: &mut String, sess: &Session) {
Expand Down Expand Up @@ -434,7 +434,7 @@ impl ModuleLlvm {
ModuleLlvm {
llmod_raw,
llcx,
tm: ManuallyDrop::new(create_informational_target_machine(tcx.sess, true)),
tm: ManuallyDrop::new(create_informational_target_machine(tcx.sess, false)),
}
}
}
Expand Down
211 changes: 107 additions & 104 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,19 +308,10 @@ pub fn check_tied_features(
/// Used to generate cfg variables and apply features
/// Must express features in the way Rust understands them
pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
let rust_features = sess
.target
.supported_target_features()
.iter()
.map(|(feature, _, _)| {
(to_llvm_features(sess, feature).llvm_feature_name, Symbol::intern(feature))
})
.collect::<FxHashMap<_, _>>();

let mut features = FxHashSet::default();

// Add base features for the target
let target_machine = create_informational_target_machine(sess, false);
let target_machine = create_informational_target_machine(sess, true);
features.extend(
sess.target
.supported_target_features()
Expand All @@ -343,13 +334,16 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
);

// Add enabled features
for llvm_feature in global_llvm_features(sess, false) {
let (add, llvm_feature) = llvm_feature.split_at(1);
let feature =
rust_features.get(llvm_feature).cloned().unwrap_or(Symbol::intern(llvm_feature));
if add == "+" {
for (enabled, feature) in
sess.opts.cg.target_feature.split(',').filter_map(|s| match s.chars().next() {
Some('+') => Some((true, Symbol::intern(&s[1..]))),
Some('-') => Some((false, Symbol::intern(&s[1..]))),
_ => None,
})
{
if enabled {
features.extend(sess.target.implied_target_features(std::iter::once(feature)));
} else if add == "-" {
} else {
features.remove(&feature);
}
}
Expand Down Expand Up @@ -475,7 +469,7 @@ fn print_target_features(out: &mut String, sess: &Session, tm: &llvm::TargetMach

pub(crate) fn print(req: &PrintRequest, mut out: &mut String, sess: &Session) {
require_inited();
let tm = create_informational_target_machine(sess, true);
let tm = create_informational_target_machine(sess, false);
match req.kind {
PrintKind::TargetCPUs => {
// SAFETY generate a C compatible string from a byte slice to pass
Expand Down Expand Up @@ -523,7 +517,11 @@ pub fn target_cpu(sess: &Session) -> &str {

/// The list of LLVM features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`,
/// `--target` and similar).
pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<String> {
pub(crate) fn global_llvm_features(
sess: &Session,
diagnostics: bool,
only_base_features: bool,
) -> Vec<String> {
// Features that come earlier are overridden by conflicting features later in the string.
// Typically we'll want more explicit settings to override the implicit ones, so:
//
Expand Down Expand Up @@ -583,96 +581,109 @@ 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
.cg
.target_feature
.split(',')
.filter_map(|s| {
let enable_disable = match s.chars().next() {
None => return None,
Some(c @ ('+' | '-')) => c,
Some(_) => {
if diagnostics {
sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature: s });
if !only_base_features {
let supported_features = sess.target.supported_target_features();
let (llvm_major, _, _) = get_version();
let mut featsmap = FxHashMap::default();
let feats = sess
.opts
.cg
.target_feature
.split(',')
.filter_map(|s| {
let enable_disable = match s.chars().next() {
None => return None,
Some(c @ ('+' | '-')) => c,
Some(_) => {
if diagnostics {
sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature: s });
}
return None;
}
return None;
}
};
};

let feature = backend_feature_name(sess, s)?;
// Warn against use of LLVM specific feature names and unstable features on the CLI.
if diagnostics {
let feature_state = supported_features.iter().find(|&&(v, _, _)| v == feature);
if feature_state.is_none() {
let rust_feature =
supported_features.iter().find_map(|&(rust_feature, _, _)| {
let llvm_features = to_llvm_features(sess, rust_feature);
if llvm_features.contains(feature)
&& !llvm_features.contains(rust_feature)
{
Some(rust_feature)
} else {
None
let feature = backend_feature_name(sess, s)?;
// Warn against use of LLVM specific feature names and unstable features on the CLI.
if diagnostics {
let feature_state = supported_features.iter().find(|&&(v, _, _)| v == feature);
if feature_state.is_none() {
let rust_feature =
supported_features.iter().find_map(|&(rust_feature, _, _)| {
let llvm_features = to_llvm_features(sess, rust_feature);
if llvm_features.contains(feature)
&& !llvm_features.contains(rust_feature)
{
Some(rust_feature)
} else {
None
}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
}
} else {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
} else if feature_state
.is_some_and(|(_name, feature_gate, _implied)| !feature_gate.is_stable())
{
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
} else {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
} else if feature_state
.is_some_and(|(_name, feature_gate, _implied)| !feature_gate.is_stable())
{
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
}
}

if diagnostics {
// FIXME(nagisa): figure out how to not allocate a full hashset here.
featsmap.insert(feature, enable_disable == '+');
}
if diagnostics {
// FIXME(nagisa): figure out how to not allocate a full hashset here.
featsmap.insert(feature, enable_disable == '+');
}

// rustc-specific features do not get passed down to LLVM…
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
return None;
}
// rustc-specific features do not get passed down to LLVM…
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 {
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
// different names when the LLVM name and the Rust name differ.
let llvm_feature = to_llvm_features(sess, feature);

Some(
std::iter::once(format!("{}{}", enable_disable, llvm_feature.llvm_feature_name))
.chain(llvm_feature.dependency.into_iter().filter_map(move |feat| {
match (enable_disable, feat) {
// 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 {
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
// different names when the LLVM name and the Rust name differ.
let llvm_feature = to_llvm_features(sess, feature);

Some(
std::iter::once(format!(
"{}{}",
enable_disable, llvm_feature.llvm_feature_name
))
.chain(llvm_feature.dependency.into_iter().filter_map(
move |feat| match (enable_disable, feat) {
('-' | '+', TargetFeatureFoldStrength::Both(f))
| ('+', TargetFeatureFoldStrength::EnableOnly(f)) => {
Some(format!("{enable_disable}{f}"))
}
_ => None,
}
})),
)
})
.flatten();
features.extend(feats);
},
)),
)
})
.flatten();
features.extend(feats);

if diagnostics && let Some(f) = check_tied_features(sess, &featsmap) {
sess.dcx().emit_err(TargetFeatureDisableOrEnable {
features: f,
span: None,
missing_features: None,
});
}
}

// -Zfixed-x18
if sess.opts.unstable_opts.fixed_x18 {
Expand All @@ -683,14 +694,6 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
}
}

if diagnostics && let Some(f) = check_tied_features(sess, &featsmap) {
sess.dcx().emit_err(TargetFeatureDisableOrEnable {
features: f,
span: None,
missing_features: None,
});
}

features
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_target/src/target_features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,13 @@ const AARCH64_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[
// FEAT_SSBS & FEAT_SSBS2
("ssbs", Stable, &[]),
// FEAT_SVE
("sve", Stable, &[]),
// It was decided that SVE requires Neon: https://github.com/rust-lang/rust/pull/91608
//
// LLVM doesn't enable Neon for SVE. ARM indicates that they're separate, but probably always
// exist together: https://developer.arm.com/documentation/102340/0100/New-features-in-SVE2
//
// "For backwards compatibility, Neon and VFP are required in the latest architectures."
("sve", Stable, &["neon"]),
// FEAT_SVE2
("sve2", Stable, &["sve"]),
// FEAT_SVE2_AES
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/target-feature-overrides.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@ revisions: COMPAT INCOMPAT
//@ needs-llvm-components: x86
//@ compile-flags: --target=x86_64-unknown-linux-gnu -Copt-level=3
//@ [COMPAT] compile-flags: -Ctarget-feature=+avx2,+avx,+sse4.2,+sse4.1,+ssse3,+sse3
//@ [COMPAT] compile-flags: -Ctarget-feature=+avx2,+avx
//@ [INCOMPAT] compile-flags: -Ctarget-feature=-avx2,-avx

// See also tests/assembly/target-feature-multiple.rs
Expand Down

0 comments on commit a25da07

Please sign in to comment.