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.

Sign up for GitHub

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

Remove ability to disable some target features #116584

Closed
wants to merge 2 commits into from
+41 −19
Closed

Remove ability to disable some target features #116584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4a7b5b
Override -C target-feature with features implied by --target
Oct 8, 2023
66625a9
Make x87, sse and sse2 mandatory on targets enabling them
Oct 8, 2023
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
38 changes: 20 additions & 18 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,23 +459,25 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
// 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:
//
// * Features disabled by --target; are overridden by
// * Features from -Ctarget-cpu=*; are overridden by [^1]
// * Features implied by --target; are overridden by
// * Features from -Ctarget-feature; are overridden by
// * Features implied by --target; are overridden by
// * function specific features.
//
// [^1]: target-cpu=native is handled here, other target-cpu values are handled implicitly
// through LLVM TargetMachine implementation.
//
// FIXME(nagisa): it isn't clear what's the best interaction between features implied by
// `-Ctarget-cpu` and `--target` are. On one hand, you'd expect CLI arguments to always
// override anything that's implicit, so e.g. when there's no `--target` flag, features implied
// the host target are overridden by `-Ctarget-cpu=*`. On the other hand, what about when both
// `--target` and `-Ctarget-cpu=*` are specified? Both then imply some target features and both
// flags are specified by the user on the CLI. It isn't as clear-cut which order of precedence
// should be taken in cases like these.
Copy link
Member

@nagisa nagisa Oct 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don’t believe this change addresses this FIXME at all. It still isn’t at all clear to me, even if we agreed that we don’t want to allow some features to be configurable outside of --target specifications, that this specific way to build up the feature set is the absolutely the correct one.

For example:

rustc -Ctarget-cpu=native --target=x86_64-unknown-linux-gnu

pretty clearly shows that the intent is for the -Ctarget-cpu=native to take precedence over whatever x86_64-unknown-linux-gnu means (including it not having any of the fancy AVX extensions.)

On the other hand

rustc -Ctarget-cpu=pentium4 --target=x86_64v4-unknown-linux-gnu

is quite a bit more ambiguous: does the user want something that would run on a pentium4? Or something that would run on an x86_64v4 class hardware, which pentium4 is not? Should the ordering of flags be taken into the account (unusual for rustc, but standard behaviour in gcc/clang style CLIs.) Perhaps it would be better to just error outright?

let mut features = vec![];

// Features disabled by an implicit or explicit `--target`.
features.extend(
sess.target
.features
.split(',')
.filter(|v| !v.is_empty() && backend_feature_name(v).is_some() && v.starts_with("-"))
.map(String::from),
);

// -Ctarget-cpu=native
match sess.opts.cg.target_cpu {
Some(ref s) if s == "native" => {
Expand All @@ -501,15 +503,6 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
Some(_) | None => {}
};

// Features implied by an implicit or explicit `--target`.
features.extend(
sess.target
.features
.split(',')
.filter(|v| !v.is_empty() && backend_feature_name(v).is_some())
.map(String::from),
);

// -Ctarget-features
let supported_features = supported_target_features(sess);
let mut featsmap = FxHashMap::default();
Expand Down Expand Up @@ -583,6 +576,15 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
.flatten();
features.extend(feats);

// Features implied by an implicit or explicit `--target`.
features.extend(
sess.target
.features
.split(',')
.filter(|v| !v.is_empty() && backend_feature_name(v).is_some() && v.starts_with("+"))
.map(String::from),
);
Copy link
Member

@RalfJung RalfJung Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

So this will silently ignore -Ctarget-features=-x87? That doesn't seem great, we shouldn't just ignore that. We should hard error or at least warn about ignoring this flag.

I think we'll want to compare that in the final feature set, certain features (for x86: x87, soft-float; for other targets we need to still determine this list) have the same state as without applying the user config. This needs to take into account -Ctarget-cpu and features implying other features.


if diagnostics && let Some(f) = check_tied_features(sess, &featsmap) {
sess.emit_err(TargetFeatureDisableOrEnable {
features: f,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i386_apple_ios.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub fn target() -> Target {
options: TargetOptions {
max_atomic_width: Some(64),
stack_probes: StackProbeType::X86,
features: "+x87".into(),
..opts("ios", arch)
},
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i386_unknown_linux_gnu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::Target;
pub fn target() -> Target {
let mut base = super::i686_unknown_linux_gnu::target();
base.cpu = "i386".into();
base.features = "+x87".into();
base.llvm_target = "i386-unknown-linux-gnu".into();
base
}
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i486_unknown_linux_gnu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::Target;
pub fn target() -> Target {
let mut base = super::i686_unknown_linux_gnu::target();
base.cpu = "i486".into();
base.features = "+x87".into();
base.llvm_target = "i486-unknown-linux-gnu".into();
base
}
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i586_pc_nto_qnx700.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub fn target() -> Target {
arch: "x86".into(),
options: TargetOptions {
cpu: "pentium4".into(),
features: "+x87".into(),
max_atomic_width: Some(64),
pre_link_args: TargetOptions::link_args(
LinkerFlavor::Gnu(Cc::Yes, Lld::No),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i586_pc_windows_msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::Target;
pub fn target() -> Target {
let mut base = super::i686_pc_windows_msvc::target();
base.cpu = "pentium".into();
base.features = "+x87".into();
base.llvm_target = "i586-pc-windows-msvc".into();
base
}
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i586_unknown_linux_gnu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::Target;
pub fn target() -> Target {
let mut base = super::i686_unknown_linux_gnu::target();
base.cpu = "pentium".into();
base.features = "+x87".into();
base.llvm_target = "i586-unknown-linux-gnu".into();
base
}
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i586_unknown_linux_musl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::Target;
pub fn target() -> Target {
let mut base = super::i686_unknown_linux_musl::target();
base.cpu = "pentium".into();
base.features = "+x87".into();
base.llvm_target = "i586-unknown-linux-musl".into();
base
}
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_apple_darwin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub fn target() -> Target {
// ld64 only understands i386 and not i686
let arch = Arch::I386;
let mut base = opts("macos", arch);
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m32"]);
base.stack_probes = StackProbeType::X86;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/spec/i686_linux_android.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub fn target() -> Target {

// https://developer.android.com/ndk/guides/abis.html#x86
base.cpu = "pentiumpro".into();
base.features = "+mmx,+sse,+sse2,+sse3,+ssse3".into();
base.features = "+x87,+mmx,+sse,+sse2,+sse3,+ssse3".into();
base.stack_probes = StackProbeType::X86;

Target {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_pc_windows_gnu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, Target};
pub fn target() -> Target {
let mut base = super::windows_gnu_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.frame_pointer = FramePointer::Always; // Required for backtraces
base.linker = Some("i686-w64-mingw32-gcc".into());
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_pc_windows_gnullvm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, Target};
pub fn target() -> Target {
let mut base = super::windows_gnullvm_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.frame_pointer = FramePointer::Always; // Required for backtraces
base.linker = Some("i686-w64-mingw32-clang".into());
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_pc_windows_msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{LinkerFlavor, Lld, Target};
pub fn target() -> Target {
let mut base = super::windows_msvc_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);

base.add_pre_link_args(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_unknown_freebsd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, LinkerFlavor, Lld, StackProbeType, Target};
pub fn target() -> Target {
let mut base = super::freebsd_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32", "-Wl,-znotext"]);
base.stack_probes = StackProbeType::X86;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_unknown_haiku.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, LinkerFlavor, Lld, StackProbeType, Target};
pub fn target() -> Target {
let mut base = super::haiku_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32"]);
base.stack_probes = StackProbeType::X86;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_unknown_linux_gnu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, LinkerFlavor, Lld, SanitizerSet, StackProbeType, Target};
pub fn target() -> Target {
let mut base = super::linux_gnu_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.supported_sanitizers = SanitizerSet::ADDRESS;
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32"]);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_unknown_linux_musl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, StackProbeType, Target};
pub fn target() -> Target {
let mut base = super::linux_musl_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32", "-Wl,-melf_i386"]);
base.stack_probes = StackProbeType::X86;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_unknown_netbsd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, LinkerFlavor, Lld, StackProbeType, Target, TargetOptions};
pub fn target() -> Target {
let mut base = super::netbsd_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32"]);
base.stack_probes = StackProbeType::X86;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_unknown_openbsd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, LinkerFlavor, Lld, StackProbeType, Target};
pub fn target() -> Target {
let mut base = super::openbsd_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32", "-fuse-ld=lld"]);
base.stack_probes = StackProbeType::X86;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_uwp_windows_gnu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, Target};
pub fn target() -> Target {
let mut base = super::windows_uwp_gnu_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.frame_pointer = FramePointer::Always; // Required for backtraces

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_uwp_windows_msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::Target;
pub fn target() -> Target {
let mut base = super::windows_uwp_msvc_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);

Target {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/i686_wrs_vxworks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{Cc, LinkerFlavor, Lld, StackProbeType, Target};
pub fn target() -> Target {
let mut base = super::vxworks_base::opts();
base.cpu = "pentium4".into();
base.features = "+x87,+sse,+sse2".into();
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m32"]);
base.stack_probes = StackProbeType::X86;
Expand Down