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 set float ABI for all ARM targets #134932

Merged
merged 4 commits into from
Dec 31, 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
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/back/owned_target_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl OwnedTargetMachine {
model: llvm::CodeModel,
reloc: llvm::RelocModel,
level: llvm::CodeGenOptLevel,
use_soft_fp: bool,
float_abi: llvm::FloatAbi,
function_sections: bool,
data_sections: bool,
unique_section_names: bool,
Expand Down Expand Up @@ -57,7 +57,7 @@ impl OwnedTargetMachine {
model,
reloc,
level,
use_soft_fp,
float_abi,
function_sections,
data_sections,
unique_section_names,
Expand Down
18 changes: 13 additions & 5 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use rustc_session::config::{
self, Lto, OutputType, Passes, RemapPathScopeComponents, SplitDwarfKind, SwitchWithOptPath,
};
use rustc_span::{BytePos, InnerSpan, Pos, SpanData, SyntaxContext, sym};
use rustc_target::spec::{CodeModel, RelocModel, SanitizerSet, SplitDebuginfo, TlsModel};
use rustc_target::spec::{CodeModel, FloatAbi, RelocModel, SanitizerSet, SplitDebuginfo, TlsModel};
use tracing::debug;

use crate::back::lto::ThinBuffer;
Expand Down Expand Up @@ -181,6 +181,14 @@ pub(crate) fn to_llvm_code_model(code_model: Option<CodeModel>) -> llvm::CodeMod
}
}

fn to_llvm_float_abi(float_abi: Option<FloatAbi>) -> llvm::FloatAbi {
match float_abi {
None => llvm::FloatAbi::Default,
Some(FloatAbi::Soft) => llvm::FloatAbi::Soft,
Some(FloatAbi::Hard) => llvm::FloatAbi::Hard,
}
}

pub(crate) fn target_machine_factory(
sess: &Session,
optlvl: config::OptLevel,
Expand All @@ -189,12 +197,12 @@ pub(crate) fn target_machine_factory(
let reloc_model = to_llvm_relocation_model(sess.relocation_model());

let (opt_level, _) = to_llvm_opt_settings(optlvl);
let use_softfp = if sess.target.arch == "arm" {
sess.opts.cg.soft_float
let float_abi = if sess.target.arch == "arm" && sess.opts.cg.soft_float {
llvm::FloatAbi::Soft
} else {
// `validate_commandline_args_with_session_available` has already warned about this being
// ignored. Let's make sure LLVM doesn't suddenly start using this flag on more targets.
false
to_llvm_float_abi(sess.target.llvm_floatabi)
};

let ffunction_sections =
Expand Down Expand Up @@ -290,7 +298,7 @@ pub(crate) fn target_machine_factory(
code_model,
reloc_model,
opt_level,
use_softfp,
float_abi,
ffunction_sections,
fdata_sections,
funique_section_names,
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ pub struct SanitizerOptions {
pub sanitize_kernel_address_recover: bool,
}

/// LLVMRelocMode
/// LLVMRustRelocModel
#[derive(Copy, Clone, PartialEq)]
#[repr(C)]
pub enum RelocModel {
Expand All @@ -538,6 +538,15 @@ pub enum RelocModel {
ROPI_RWPI,
}

/// LLVMRustFloatABI
#[derive(Copy, Clone, PartialEq)]
#[repr(C)]
pub enum FloatAbi {
Default,
Soft,
Hard,
}

/// LLVMRustCodeModel
#[derive(Copy, Clone)]
#[repr(C)]
Expand Down Expand Up @@ -2192,7 +2201,7 @@ unsafe extern "C" {
Model: CodeModel,
Reloc: RelocModel,
Level: CodeGenOptLevel,
UseSoftFP: bool,
FloatABIType: FloatAbi,
FunctionSections: bool,
DataSections: bool,
UniqueSectionNames: bool,
Expand Down
26 changes: 21 additions & 5 deletions compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,24 @@ static Reloc::Model fromRust(LLVMRustRelocModel RustReloc) {
report_fatal_error("Bad RelocModel.");
}

enum class LLVMRustFloatABI {
Default,
Soft,
Hard,
};

static FloatABI::ABIType fromRust(LLVMRustFloatABI RustFloatAbi) {
switch (RustFloatAbi) {
case LLVMRustFloatABI::Default:
return FloatABI::Default;
case LLVMRustFloatABI::Soft:
return FloatABI::Soft;
case LLVMRustFloatABI::Hard:
return FloatABI::Hard;
}
report_fatal_error("Bad FloatABI.");
}

/// getLongestEntryLength - Return the length of the longest entry in the table.
template <typename KV> static size_t getLongestEntryLength(ArrayRef<KV> Table) {
size_t MaxLen = 0;
Expand Down Expand Up @@ -358,7 +376,7 @@ extern "C" const char *LLVMRustGetHostCPUName(size_t *OutLen) {
extern "C" LLVMTargetMachineRef LLVMRustCreateTargetMachine(
const char *TripleStr, const char *CPU, const char *Feature,
const char *ABIStr, LLVMRustCodeModel RustCM, LLVMRustRelocModel RustReloc,
LLVMRustCodeGenOptLevel RustOptLevel, bool UseSoftFloat,
LLVMRustCodeGenOptLevel RustOptLevel, LLVMRustFloatABI RustFloatABIType,
bool FunctionSections, bool DataSections, bool UniqueSectionNames,
bool TrapUnreachable, bool Singlethread, bool VerboseAsm,
bool EmitStackSizeSection, bool RelaxELFRelocations, bool UseInitArray,
Expand All @@ -369,6 +387,7 @@ extern "C" LLVMTargetMachineRef LLVMRustCreateTargetMachine(
auto OptLevel = fromRust(RustOptLevel);
auto RM = fromRust(RustReloc);
auto CM = fromRust(RustCM);
auto FloatABIType = fromRust(RustFloatABIType);

std::string Error;
auto Trip = Triple(Triple::normalize(TripleStr));
Expand All @@ -381,10 +400,7 @@ extern "C" LLVMTargetMachineRef LLVMRustCreateTargetMachine(

TargetOptions Options = codegen::InitTargetOptionsFromCodeGenFlags(Trip);

Options.FloatABIType = FloatABI::Default;
if (UseSoftFloat) {
Options.FloatABIType = FloatABI::Soft;
}
Options.FloatABIType = FloatABIType;
Options.DataSections = DataSections;
Options.FunctionSections = FunctionSections;
Options.UniqueSectionNames = UniqueSectionNames;
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_target/src/spec/base/apple/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::borrow::Cow;
use std::env;

use crate::spec::{
Cc, DebuginfoKind, FramePointer, LinkerFlavor, Lld, SplitDebuginfo, StackProbeType, StaticCow,
TargetOptions, cvs,
Cc, DebuginfoKind, FloatAbi, FramePointer, LinkerFlavor, Lld, SplitDebuginfo, StackProbeType,
StaticCow, TargetOptions, cvs,
};

#[cfg(test)]
Expand Down Expand Up @@ -105,6 +105,7 @@ pub(crate) fn base(
) -> (TargetOptions, StaticCow<str>, StaticCow<str>) {
let opts = TargetOptions {
abi: abi.target_abi().into(),
llvm_floatabi: Some(FloatAbi::Hard),
Copy link
Member Author

@RalfJung RalfJung Dec 30, 2024

Choose a reason for hiding this comment

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

I assume all the Apple targets use a hardfloat ABI? This is about armv7s_apple_ios.rs and armv7k_apple_watchos.rs. Both of these set +neon in their target features. LLVM switches to hardfloats for (TargetTriple.isOSBinFormatMachO() && TargetTriple.getSubArch() == Triple::ARMSubArch_v7em), not sure if that applies here.

Cc @deg4uss3r @madsmtm

Copy link
Contributor

Choose a reason for hiding this comment

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

That is my understanding, yeah, and matches what Clang reports when you try to use a different float ABI:

$ touch foo.c
$ xcrun -sdk watchos clang -target armv7k-apple-watchos -S -c foo.c -mfloat-abi=hard # Succeeds
$ xcrun -sdk watchos clang -target armv7k-apple-watchos -S -c foo.c -mfloat-abi=soft # Fails
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7k-apple-watchos'
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7k-apple-watchos'
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7k-apple-watchos'
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7k-apple-watchos'
$ xcrun -sdk watchos clang -target armv7s-apple-watchos -S -c foo.c -mfloat-abi=hard # Succeeds
$ xcrun -sdk watchos clang -target armv7s-apple-watchos -S -c foo.c -mfloat-abi=soft # Fails
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7s-apple-watchos'
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7s-apple-watchos'
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7s-apple-watchos'
clang: error: unsupported option '-mfloat-abi=soft' for target 'thumbv7s-apple-watchos'

os: os.into(),
cpu: arch.target_cpu(abi).into(),
link_env_remove: link_env_remove(os),
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_target/src/spec/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ impl Target {
Some(Ok(()))
})).unwrap_or(Ok(()))
} );
($key_name:ident, FloatAbi) => ( {
let name = (stringify!($key_name)).replace("_", "-");
obj.remove(&name).and_then(|o| o.as_str().and_then(|s| {
match s.parse::<super::FloatAbi>() {
Ok(float_abi) => base.$key_name = Some(float_abi),
_ => return Some(Err(format!("'{}' is not a valid value for \
llvm-floatabi. Use 'soft' or 'hard'.",
s))),
}
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 @@ -598,6 +610,7 @@ impl Target {
key!(mcount = "target-mcount");
key!(llvm_mcount_intrinsic, optional);
key!(llvm_abiname);
key!(llvm_floatabi, FloatAbi)?;
key!(relax_elf_relocations, bool);
key!(llvm_args, list);
key!(use_ctors_section, bool);
Expand Down Expand Up @@ -772,6 +785,7 @@ impl ToJson for Target {
target_option_val!(mcount, "target-mcount");
target_option_val!(llvm_mcount_intrinsic);
target_option_val!(llvm_abiname);
target_option_val!(llvm_floatabi);
target_option_val!(relax_elf_relocations);
target_option_val!(llvm_args);
target_option_val!(use_ctors_section);
Expand Down
47 changes: 46 additions & 1 deletion compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,35 @@ impl ToJson for CodeModel {
}
}

/// The float ABI setting to be configured in the LLVM target machine.
#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum FloatAbi {
Soft,
Hard,
Comment on lines +1091 to +1092
Copy link
Member

Choose a reason for hiding this comment

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

Some documentation about what each variant mean would be great ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what to say here other than "use softfloat ABI" vs "use hardfloat ABI", which seems a bit pointless.

Copy link
Member

Choose a reason for hiding this comment

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

We could hypothetically say something about what "soft float" and "hard float" mean, but I think that is something we should consider as a more general topic.

}

impl FromStr for FloatAbi {
type Err = ();

fn from_str(s: &str) -> Result<FloatAbi, ()> {
Ok(match s {
"soft" => FloatAbi::Soft,
"hard" => FloatAbi::Hard,
_ => return Err(()),
})
}
}

impl ToJson for FloatAbi {
fn to_json(&self) -> Json {
match *self {
FloatAbi::Soft => "soft",
FloatAbi::Hard => "hard",
}
.to_json()
}
}

#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum TlsModel {
GeneralDynamic,
Expand Down Expand Up @@ -2150,6 +2179,8 @@ pub struct TargetOptions {
pub env: StaticCow<str>,
/// ABI name to distinguish multiple ABIs on the same OS and architecture. For instance, `"eabi"`
/// or `"eabihf"`. Defaults to "".
/// This field is *not* forwarded directly to LLVM; its primary purpose is `cfg(target_abi)`.
/// However, parts of the backend do check this field for specific values to enable special behavior.
pub abi: StaticCow<str>,
/// Vendor name to use for conditional compilation (`target_vendor`). Defaults to "unknown".
pub vendor: StaticCow<str>,
Expand Down Expand Up @@ -2446,8 +2477,17 @@ pub struct TargetOptions {
pub llvm_mcount_intrinsic: Option<StaticCow<str>>,

/// LLVM ABI name, corresponds to the '-mabi' parameter available in multilib C compilers
/// and the `-target-abi` flag in llc. In the LLVM API this is `MCOptions.ABIName`.
pub llvm_abiname: StaticCow<str>,

/// Control the float ABI to use, for architectures that support it. The only architecture we
/// currently use this for is ARM. Corresponds to the `-float-abi` flag in llc. In the LLVM API
/// this is `FloatABIType`. (clang's `-mfloat-abi` is similar but more complicated since it
/// can also affect the `soft-float` target feature.)
///
/// If not provided, LLVM will infer the float ABI from the target triple (`llvm_target`).
pub llvm_floatabi: Option<FloatAbi>,

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

Expand Down Expand Up @@ -2719,6 +2759,7 @@ impl Default for TargetOptions {
mcount: "mcount".into(),
llvm_mcount_intrinsic: None,
llvm_abiname: "".into(),
llvm_floatabi: None,
relax_elf_relocations: false,
llvm_args: cvs![],
use_ctors_section: false,
Expand Down Expand Up @@ -3153,7 +3194,8 @@ impl Target {
);
}

// Check that RISC-V targets always specify which ABI they use.
// Check that RISC-V targets always specify which ABI they use,
// and that ARM targets specify their float ABI.
match &*self.arch {
"riscv32" => {
check_matches!(
Expand All @@ -3170,6 +3212,9 @@ impl Target {
"invalid RISC-V ABI name"
);
}
"arm" => {
check!(self.llvm_floatabi.is_some(), "ARM targets must specify their float ABI",)
}
_ => {}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{SanitizerSet, Target, TargetOptions, base};
use crate::spec::{FloatAbi, SanitizerSet, Target, TargetOptions, base};

pub(crate) fn target() -> Target {
Target {
Expand All @@ -14,6 +14,7 @@ pub(crate) fn target() -> Target {
arch: "arm".into(),
options: TargetOptions {
abi: "eabi".into(),
llvm_floatabi: Some(FloatAbi::Soft),
// https://developer.android.com/ndk/guides/abis.html#armeabi
features: "+strict-align,+v5te".into(),
supported_sanitizers: SanitizerSet::ADDRESS,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{Target, TargetOptions, base};
use crate::spec::{FloatAbi, Target, TargetOptions, base};

pub(crate) fn target() -> Target {
Target {
Expand All @@ -14,6 +14,7 @@ pub(crate) fn target() -> Target {
arch: "arm".into(),
options: TargetOptions {
abi: "eabi".into(),
llvm_floatabi: Some(FloatAbi::Soft),
features: "+strict-align,+v6".into(),
max_atomic_width: Some(64),
mcount: "\u{1}__gnu_mcount_nc".into(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{Target, TargetOptions, base};
use crate::spec::{FloatAbi, Target, TargetOptions, base};

pub(crate) fn target() -> Target {
Target {
Expand All @@ -14,6 +14,7 @@ pub(crate) fn target() -> Target {
arch: "arm".into(),
options: TargetOptions {
abi: "eabihf".into(),
llvm_floatabi: Some(FloatAbi::Hard),
features: "+strict-align,+v6,+vfp2,-d32".into(),
max_atomic_width: Some(64),
mcount: "\u{1}__gnu_mcount_nc".into(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use crate::spec::{Target, TargetOptions, base};
use crate::spec::{FloatAbi, Target, TargetOptions, base};

pub(crate) fn target() -> Target {
Target {
// It's important we use "gnueabi" and not "musleabi" here. LLVM uses it
// to determine the calling convention and float ABI, and it doesn't
// support the "musleabi" value.
llvm_target: "arm-unknown-linux-gnueabi".into(),
llvm_target: "arm-unknown-linux-musleabi".into(),
metadata: crate::spec::TargetMetadata {
description: Some("Armv6 Linux with musl 1.2.3".into()),
tier: Some(2),
Expand All @@ -17,6 +14,7 @@ pub(crate) fn target() -> Target {
arch: "arm".into(),
options: TargetOptions {
abi: "eabi".into(),
llvm_floatabi: Some(FloatAbi::Soft),
// Most of these settings are copied from the arm_unknown_linux_gnueabi
// target.
features: "+strict-align,+v6".into(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use crate::spec::{Target, TargetOptions, base};
use crate::spec::{FloatAbi, Target, TargetOptions, base};

pub(crate) fn target() -> Target {
Target {
// It's important we use "gnueabihf" and not "musleabihf" here. LLVM
// uses it to determine the calling convention and float ABI, and it
// doesn't support the "musleabihf" value.
llvm_target: "arm-unknown-linux-gnueabihf".into(),
llvm_target: "arm-unknown-linux-musleabihf".into(),
metadata: crate::spec::TargetMetadata {
description: Some("Armv6 Linux with musl 1.2.3, hardfloat".into()),
tier: Some(2),
Expand All @@ -17,6 +14,7 @@ pub(crate) fn target() -> Target {
arch: "arm".into(),
options: TargetOptions {
abi: "eabihf".into(),
llvm_floatabi: Some(FloatAbi::Hard),
// Most of these settings are copied from the arm_unknown_linux_gnueabihf
// target.
features: "+strict-align,+v6,+vfp2,-d32".into(),
Expand Down
Loading
Loading