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

Explicitly choose x86 softfloat/hardfloat ABI #136146

Merged
merged 2 commits into from
Feb 3, 2025
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
15 changes: 15 additions & 0 deletions compiler/rustc_target/src/spec/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,19 @@ impl Target {
Some(Ok(()))
})).unwrap_or(Ok(()))
} );
($key_name:ident, RustcAbi) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.remove(&name).and_then(|o| o.as_str().and_then(|s| {
match s.parse::<super::RustcAbi>() {
Ok(rustc_abi) => base.$key_name = Some(rustc_abi),
_ => return Some(Err(format!(
"'{s}' is not a valid value for rustc-abi. \
Use 'x86-softfloat' or leave the field unset."
))),
}
Some(Ok(()))
})).unwrap_or(Ok(()))
} );
($key_name:ident, RelocModel) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.remove(&name).and_then(|o| o.as_str().and_then(|s| {
Expand Down Expand Up @@ -611,6 +624,7 @@ impl Target {
key!(llvm_mcount_intrinsic, optional);
key!(llvm_abiname);
key!(llvm_floatabi, FloatAbi)?;
key!(rustc_abi, RustcAbi)?;
key!(relax_elf_relocations, bool);
key!(llvm_args, list);
key!(use_ctors_section, bool);
Expand Down Expand Up @@ -786,6 +800,7 @@ impl ToJson for Target {
target_option_val!(llvm_mcount_intrinsic);
target_option_val!(llvm_abiname);
target_option_val!(llvm_floatabi);
target_option_val!(rustc_abi);
target_option_val!(relax_elf_relocations);
target_option_val!(llvm_args);
target_option_val!(use_ctors_section);
Expand Down
49 changes: 45 additions & 4 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,33 @@ impl ToJson for FloatAbi {
}
}

/// The Rustc-specific variant of the ABI used for this target.
#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum RustcAbi {
/// On x86-32/64 only: do not use any FPU or SIMD registers for the ABI.
X86Softfloat,
}

impl FromStr for RustcAbi {
type Err = ();

fn from_str(s: &str) -> Result<RustcAbi, ()> {
Ok(match s {
"x86-softfloat" => RustcAbi::X86Softfloat,
_ => return Err(()),
})
}
}

impl ToJson for RustcAbi {
fn to_json(&self) -> Json {
match *self {
RustcAbi::X86Softfloat => "x86-softfloat",
}
.to_json()
}
}

#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum TlsModel {
GeneralDynamic,
Expand Down Expand Up @@ -2502,6 +2529,12 @@ pub struct TargetOptions {
/// If not provided, LLVM will infer the float ABI from the target triple (`llvm_target`).
pub llvm_floatabi: Option<FloatAbi>,

/// Picks a specific ABI for this target. This is *not* just for "Rust" ABI functions,
/// it can also affect "C" ABI functions; the point is that this flag is interpreted by
/// rustc and not forwarded to LLVM.
/// So far, this is only used on x86.
pub rustc_abi: Option<RustcAbi>,

/// Whether or not RelaxElfRelocation flag will be passed to the linker
pub relax_elf_relocations: bool,

Expand Down Expand Up @@ -2661,10 +2694,6 @@ impl TargetOptions {
.collect();
}
}

pub(crate) fn has_feature(&self, search_feature: &str) -> bool {
self.features.split(',').any(|f| f.strip_prefix('+').is_some_and(|f| f == search_feature))
}
}

impl Default for TargetOptions {
Expand Down Expand Up @@ -2770,6 +2799,7 @@ impl Default for TargetOptions {
llvm_mcount_intrinsic: None,
llvm_abiname: "".into(),
llvm_floatabi: None,
rustc_abi: None,
relax_elf_relocations: false,
llvm_args: cvs![],
use_ctors_section: false,
Expand Down Expand Up @@ -3236,6 +3266,17 @@ impl Target {
_ => {}
}

// Check consistency of Rust ABI declaration.
if let Some(rust_abi) = self.rustc_abi {
match rust_abi {
RustcAbi::X86Softfloat => check_matches!(
&*self.arch,
"x86" | "x86_64",
"`x86-softfloat` ABI is only valid for x86 targets"
),
}
}

// Check that the given target-features string makes some basic sense.
if !self.features.is_empty() {
let mut features_enabled = FxHashSet::default();
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/spec/targets/i686_unknown_uefi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// The cdecl ABI is used. It differs from the stdcall or fastcall ABI.
// "i686-unknown-windows" is used to get the minimal subset of windows-specific features.

use crate::spec::{Target, base};
use crate::spec::{RustcAbi, Target, base};

pub(crate) fn target() -> Target {
let mut base = base::uefi_msvc::opts();
Expand All @@ -22,6 +22,7 @@ pub(crate) fn target() -> Target {
// If you initialize FP units yourself, you can override these flags with custom linker
// arguments, thus giving you access to full MMX/SSE acceleration.
base.features = "-mmx,-sse,+soft-float".into();
base.rustc_abi = Some(RustcAbi::X86Softfloat);

// Use -GNU here, because of the reason below:
// Background and Problem:
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_target/src/spec/targets/x86_64_unknown_none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
// features.

use crate::spec::{
Cc, CodeModel, LinkerFlavor, Lld, PanicStrategy, RelroLevel, SanitizerSet, StackProbeType,
Target, TargetOptions,
Cc, CodeModel, LinkerFlavor, Lld, PanicStrategy, RelroLevel, RustcAbi, SanitizerSet,
StackProbeType, Target, TargetOptions,
};

pub(crate) fn target() -> Target {
Expand All @@ -20,6 +20,7 @@ pub(crate) fn target() -> Target {
relro_level: RelroLevel::Full,
linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes),
linker: Some("rust-lld".into()),
rustc_abi: Some(RustcAbi::X86Softfloat),
features: "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-avx,-avx2,+soft-float".into(),
supported_sanitizers: SanitizerSet::KCFI | SanitizerSet::KERNELADDRESS,
disable_redzone: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// LLVM. "x86_64-unknown-windows" is used to get the minimal subset of windows-specific features.

use crate::abi::call::Conv;
use crate::spec::{Target, base};
use crate::spec::{RustcAbi, Target, base};

pub(crate) fn target() -> Target {
let mut base = base::uefi_msvc::opts();
Expand All @@ -26,6 +26,7 @@ pub(crate) fn target() -> Target {
// If you initialize FP units yourself, you can override these flags with custom linker
// arguments, thus giving you access to full MMX/SSE acceleration.
base.features = "-mmx,-sse,+soft-float".into();
base.rustc_abi = Some(RustcAbi::X86Softfloat);

Target {
llvm_target: "x86_64-unknown-windows".into(),
Expand Down
52 changes: 36 additions & 16 deletions compiler/rustc_target/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_span::{Symbol, sym};

use crate::spec::{FloatAbi, Target};
use crate::spec::{FloatAbi, RustcAbi, Target};

/// Features that control behaviour of rustc, rather than the codegen.
/// These exist globally and are not in the target-specific lists below.
Expand Down Expand Up @@ -417,7 +417,9 @@ const X86_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[
("sha512", Unstable(sym::sha512_sm_x86), &["avx2"]),
("sm3", Unstable(sym::sha512_sm_x86), &["avx"]),
("sm4", Unstable(sym::sha512_sm_x86), &["avx2"]),
("soft-float", Stability::Forbidden { reason: "unsound because it changes float ABI" }, &[]),
// This cannot actually be toggled, the ABI always fixes it, so it'd make little sense to
// stabilize. It must be in this list for the ABI check to be able to use it.
("soft-float", Stability::Unstable(sym::x87_target_feature), &[]),
Copy link
Member Author

Choose a reason for hiding this comment

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

Making this "forbidden" just leads to duplicate warnings so there's little point in doing that.

This makes riscv's forced-atomics the only remaining "forbidden" feature.

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense to me.

("sse", Stable, &[]),
("sse2", Stable, &["sse"]),
("sse3", Stable, &["sse2"]),
Expand Down Expand Up @@ -768,23 +770,41 @@ impl Target {
// questions "which ABI is used".
match &*self.arch {
"x86" => {
// We support 2 ABIs, hardfloat (default) and softfloat.
// x86 has no sane ABI indicator so we have to use the target feature.
if self.has_feature("soft-float") {
NOTHING
} else {
// Hardfloat ABI. x87 must be enabled.
FeatureConstraints { required: &["x87"], incompatible: &[] }
// We use our own ABI indicator here; LLVM does not have anything native.
// Every case should require or forbid `soft-float`!
match self.rustc_abi {
None => {
// Default hardfloat ABI.
// x87 must be enabled, soft-float must be disabled.
FeatureConstraints { required: &["x87"], incompatible: &["soft-float"] }
}
Some(RustcAbi::X86Softfloat) => {
// Softfloat ABI, requires corresponding target feature. That feature trumps
// `x87` and all other FPU features so those do not matter.
// Note that this one requirement is the entire implementation of the ABI!
// LLVM handles the rest.
FeatureConstraints { required: &["soft-float"], incompatible: &[] }
}
}
}
"x86_64" => {
// We support 2 ABIs, hardfloat (default) and softfloat.
// x86 has no sane ABI indicator so we have to use the target feature.
if self.has_feature("soft-float") {
NOTHING
} else {
// Hardfloat ABI. x87 and SSE2 must be enabled.
FeatureConstraints { required: &["x87", "sse2"], incompatible: &[] }
// We use our own ABI indicator here; LLVM does not have anything native.
// Every case should require or forbid `soft-float`!
match self.rustc_abi {
None => {
// Default hardfloat ABI. On x86-64, this always includes SSE2.
FeatureConstraints {
required: &["x87", "sse2"],
incompatible: &["soft-float"],
}
}
Some(RustcAbi::X86Softfloat) => {
// Softfloat ABI, requires corresponding target feature. That feature trumps
// `x87` and all other FPU features so those do not matter.
// Note that this one requirement is the entire implementation of the ABI!
// LLVM handles the rest.
FeatureConstraints { required: &["soft-float"], incompatible: &[] }
}
}
}
"arm" => {
Expand Down
4 changes: 2 additions & 2 deletions src/ci/docker/scripts/rfl-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

set -euo pipefail

LINUX_VERSION=v6.13-rc1
LINUX_VERSION=50e57739141b41f731ab31f8380821c7969f9dc4

# Build rustc, rustdoc, cargo, clippy-driver and rustfmt
../x.py build --stage 2 library rustdoc clippy rustfmt
Expand All @@ -28,7 +28,7 @@ rm -rf linux || true
# Download Linux at a specific commit
mkdir -p linux
git -C linux init
git -C linux remote add origin https://github.com/Rust-for-Linux/linux.git
git -C linux remote add origin https://github.com/Darksonn/linux.git
Copy link

Choose a reason for hiding this comment

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

this needs a cfg(bootstrap) to get bumped on release

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't follow, what does the Linux kernel have to do with bootstraping?

I think I did everything mentioned at https://rustc-dev-guide.rust-lang.org/tests/rust-for-linux.html.

Copy link

Choose a reason for hiding this comment

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

Oops, I missed that the maintainers have taken responsibility for flipping this back and don't need extra reminders from Release.

Copy link
Contributor

@ojeda ojeda Feb 9, 2025

Choose a reason for hiding this comment

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

I don't follow, what does the Linux kernel have to do with bootstraping?

I think I did everything mentioned at https://rustc-dev-guide.rust-lang.org/tests/rust-for-linux.html.

I am confused too -- this seems fine.

When v6.14-rc3 releases in a couple weeks, which is the one I am planning to get the fix into, I will send a PR with that tag, and we will be in the normal situation again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, I missed that the maintainers have taken responsibility for flipping this back and don't need extra reminders from Release.

I don't think there is a technical requirement to flip it back. I mean, the idea is that we do so, but it is orthogonal to the release, no? It is just a CI test. For instance, in an ideal world, we could be testing several supported Linux versions (e.g. the latest LTS).

git -C linux fetch --depth 1 origin ${LINUX_VERSION}
git -C linux checkout FETCH_HEAD

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//! Ensure ABI-incompatible features cannot be enabled via `#[target_feature]`.
//@ compile-flags: --target=riscv32e-unknown-none-elf --crate-type=lib
//@ needs-llvm-components: riscv
#![feature(no_core, lang_items, riscv_target_feature)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: target feature `d` cannot be enabled with `#[target_feature]`: this feature is incompatible with the target ABI
--> $DIR/forbidden-hardfloat-target-feature-attribute.rs:9:18
--> $DIR/forbidden-hardfloat-target-feature-attribute.rs:10:18
|
LL | #[target_feature(enable = "d")]
| ^^^^^^^^^^^^
Expand Down
14 changes: 0 additions & 14 deletions tests/ui/target-feature/forbidden-hardfloat-target-feature-cfg.rs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
//! Ensure that if disabling a target feature implies disabling an ABI-required target feature,
//! we complain.
//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib
//@ needs-llvm-components: x86
//@ compile-flags: -Ctarget-feature=-sse
// For now this is just a warning.
//@ build-pass
//@error-pattern: must be enabled to ensure that the ABI
#![feature(no_core, lang_items)]
#![no_core]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//@ compile-flags: -Ctarget-feature=-neon
// For now this is just a warning.
//@ build-pass
//@error-pattern: must be enabled to ensure that the ABI
#![feature(no_core, lang_items)]
#![no_core]

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//! Ensure ABI-required features cannot be disabled via `-Ctarget-feature`.
//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib
//@ needs-llvm-components: x86
//@ compile-flags: -Ctarget-feature=-x87
// For now this is just a warning.
//@ build-pass
//@error-pattern: must be enabled to ensure that the ABI
#![feature(no_core, lang_items)]
#![no_core]

Expand Down
12 changes: 12 additions & 0 deletions tests/ui/target-feature/forbidden-hardfloat-target-feature-flag.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//! Ensure ABI-incompatible features cannot be enabled via `-Ctarget-feature`.
//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib
//@ needs-llvm-components: x86
//@ compile-flags: -Ctarget-feature=+soft-float
// For now this is just a warning.
//@ build-pass
//@error-pattern: must be disabled to ensure that the ABI
#![feature(no_core, lang_items, riscv_target_feature)]
#![no_core]

#[lang = "sized"]
pub trait Sized {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
warning: target feature `soft-float` must be disabled to ensure that the ABI of the current target can be implemented correctly
|
= note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

warning: unstable feature specified for `-Ctarget-feature`: `soft-float`
|
= note: this feature is not stably supported; its behavior can change in the future

warning: 2 warnings emitted

7 changes: 4 additions & 3 deletions tests/ui/target-feature/forbidden-target-feature-attribute.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib
//@ needs-llvm-components: x86
//! Ensure "forbidden" target features cannot be enabled via `#[target_feature]`.
//@ compile-flags: --target=riscv32e-unknown-none-elf --crate-type=lib
Copy link
Member

Choose a reason for hiding this comment

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

thanks for better documenting these!

//@ needs-llvm-components: riscv
#![feature(no_core, lang_items)]
#![no_core]

#[lang = "sized"]
pub trait Sized {}

#[target_feature(enable = "soft-float")]
#[target_feature(enable = "forced-atomics")]
//~^ERROR: cannot be enabled with
pub unsafe fn my_fun() {}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: target feature `soft-float` cannot be enabled with `#[target_feature]`: unsound because it changes float ABI
--> $DIR/forbidden-target-feature-attribute.rs:9:18
error: target feature `forced-atomics` cannot be enabled with `#[target_feature]`: unsound because it changes the ABI of atomic operations
--> $DIR/forbidden-target-feature-attribute.rs:10:18
|
LL | #[target_feature(enable = "soft-float")]
| ^^^^^^^^^^^^^^^^^^^^^
LL | #[target_feature(enable = "forced-atomics")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

9 changes: 5 additions & 4 deletions tests/ui/target-feature/forbidden-target-feature-cfg.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ compile-flags: --target=x86_64-unknown-none --crate-type=lib
//@ needs-llvm-components: x86
//! Ensure "forbidden" target features are not exposed via `cfg`.
//@ compile-flags: --target=riscv32e-unknown-none-elf --crate-type=lib
//@ needs-llvm-components: riscv
//@ check-pass
#![feature(no_core, lang_items)]
#![no_core]
Expand All @@ -10,5 +11,5 @@ pub trait Sized {}

// The compile_error macro does not exist, so if the `cfg` evaluates to `true` this
// complains about the missing macro rather than showing the error... but that's good enough.
#[cfg(target_feature = "soft-float")]
compile_error!("the soft-float feature should not be exposed in `cfg`");
#[cfg(target_feature = "forced-atomics")]
compile_error!("the forced-atomics feature should not be exposed in `cfg`");
Loading
Loading