explicitly set float ABI for all ARM targets #134932

RalfJung · 2024-12-30T18:54:49Z

We currently always set the FloatABIType field in the LLVM target machine to Default, which means LLVM infers the ARM float ABI (hard vs soft) from the LLVM target triple. This causes problems such as having to set the LLVM triple to *-gnueabi for our musleabi targets to ensure they get correctly inferred as soft-float targets. It also means rustc doesn't really know which float ABI ends up being used, which is a blocker for #134794. So I think we should stop doing that and instead explicitly control that value. That's what this PR implements.

See Zulip for more context.

Best reviewed commit-by-commit. I hope I got all those llvm_floatabi values right...

rustbot · 2024-12-30T18:54:58Z

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-12-30T18:55:00Z

These commits modify compiler targets.
(See the Target Tier Policy.)

RalfJung · 2024-12-30T18:57:08Z

compiler/rustc_target/src/spec/targets/thumbv7a_pc_windows_msvc.rs

@@ -23,6 +23,7 @@ pub(crate) fn target() -> Target {
        data_layout: "e-m:w-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(),
        arch: "arm".into(),
        options: TargetOptions {
+            llvm_floatabi: Some(FloatAbi::Hard),


I assume Windows uses a hardfloat ABI? The abi field is left empty it seems.

RalfJung · 2024-12-30T18:57:17Z

compiler/rustc_target/src/spec/targets/thumbv7a_uwp_windows_msvc.rs

@@ -13,6 +13,7 @@ pub(crate) fn target() -> Target {
        data_layout: "e-m:w-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(),
        arch: "arm".into(),
        options: TargetOptions {
+            llvm_floatabi: Some(FloatAbi::Hard),


Same here re: Windows

RalfJung · 2024-12-30T19:00:29Z

compiler/rustc_target/src/spec/base/apple/mod.rs

@@ -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),


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

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'

Urgau · 2024-12-30T19:01:55Z

compiler/rustc_target/src/spec/mod.rs

+    Soft,
+    Hard,


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

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

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.

compiler/rustc_target/src/spec/mod.rs

compiler/rustc_target/src/spec/json.rs

compiler/rustc_target/src/spec/mod.rs

…t we set the float ABI explicitly

workingjubilee

Yeah, this is fine.

workingjubilee · 2024-12-30T21:46:54Z

@bors r+ rollup

bors · 2024-12-30T21:46:57Z

📌 Commit c3189c5 has been approved by workingjubilee

It is now in the queue for this repository.

…ubilee explicitly set float ABI for all ARM targets We currently always set the `FloatABIType` field in the LLVM target machine to `Default`, which means LLVM infers the ARM float ABI (hard vs soft) from the LLVM target triple. This causes problems such as having to set the LLVM triple to `*-gnueabi` for our `musleabi` targets to ensure they get correctly inferred as soft-float targets. It also means rustc doesn't really know which float ABI ends up being used, which is a blocker for rust-lang#134794. So I think we should stop doing that and instead explicitly control that value. That's what this PR implements. See [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/187780-t-compiler.2Fwg-llvm/topic/Softfloat.20ABI.2C.20hardfloat.20instructions) for more context. Best reviewed commit-by-commit. I hope I got all those `llvm_floatabi` values right...

Rollup of 7 pull requests Successful merges: - rust-lang#133461 (Add COPYRIGHT-*.html files to distribution and update `COPYRIGHT`) - rust-lang#134919 (bootstrap: Make `./x test compiler` actually run the compiler unit tests) - rust-lang#134927 (Make slice::as_flattened_mut unstably const) - rust-lang#134930 (ptr docs: make it clear that we are talking only about memory accesses) - rust-lang#134932 (explicitly set float ABI for all ARM targets) - rust-lang#134934 (Fix typos) - rust-lang#134941 (compiler: Add a statement-of-intent to `rustc_abi`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - rust-lang#134919 (bootstrap: Make `./x test compiler` actually run the compiler unit tests) - rust-lang#134927 (Make slice::as_flattened_mut unstably const) - rust-lang#134930 (ptr docs: make it clear that we are talking only about memory accesses) - rust-lang#134932 (explicitly set float ABI for all ARM targets) - rust-lang#134933 (Make sure we check the future type is `Sized` in `AsyncFn*`) - rust-lang#134934 (Fix typos) - rust-lang#134941 (compiler: Add a statement-of-intent to `rustc_abi`) - rust-lang#134949 (Convert some `Into` impls into `From` impls) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#134932 - RalfJung:arm-float-abi, r=workingjubilee explicitly set float ABI for all ARM targets We currently always set the `FloatABIType` field in the LLVM target machine to `Default`, which means LLVM infers the ARM float ABI (hard vs soft) from the LLVM target triple. This causes problems such as having to set the LLVM triple to `*-gnueabi` for our `musleabi` targets to ensure they get correctly inferred as soft-float targets. It also means rustc doesn't really know which float ABI ends up being used, which is a blocker for rust-lang#134794. So I think we should stop doing that and instead explicitly control that value. That's what this PR implements. See [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/187780-t-compiler.2Fwg-llvm/topic/Softfloat.20ABI.2C.20hardfloat.20instructions) for more context. Best reviewed commit-by-commit. I hope I got all those `llvm_floatabi` values right...

rustc_llvm: expose FloatABIType target machine parameter

fff026c

rustbot assigned chenyukang Dec 30, 2024

rustbot added O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 30, 2024

RalfJung mentioned this pull request Dec 30, 2024

Add a notion of "some ABIs require certain target features" #134794

Open

RalfJung commented Dec 30, 2024

View reviewed changes

Urgau reviewed Dec 30, 2024

View reviewed changes

RalfJung force-pushed the arm-float-abi branch 3 times, most recently from 2b57cdf to 396c765 Compare December 30, 2024 19:09

maurer approved these changes Dec 30, 2024

View reviewed changes

compiler/rustc_target/src/spec/json.rs Show resolved Hide resolved

compiler/rustc_target/src/spec/mod.rs Outdated Show resolved Hide resolved

madsmtm approved these changes Dec 30, 2024

View reviewed changes

RalfJung force-pushed the arm-float-abi branch 3 times, most recently from 9f134ad to bee752c Compare December 30, 2024 20:57

RalfJung added 3 commits December 30, 2024 21:59

add llvm_floatabi field to target spec that controls FloatABIType

a0dbb37

explicitly set float ABI for all ARM targets

a51fefc

musleabi* targets: we no longer need to set gnueabi* for LLVM now tha…

c3189c5

…t we set the float ABI explicitly

RalfJung force-pushed the arm-float-abi branch from bee752c to c3189c5 Compare December 30, 2024 20:59

workingjubilee assigned workingjubilee and unassigned chenyukang Dec 30, 2024

workingjubilee approved these changes Dec 30, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 30, 2024

Zalathar mentioned this pull request Dec 31, 2024

Rollup of 7 pull requests #134944

Closed

Zalathar mentioned this pull request Dec 31, 2024

Rollup of 8 pull requests #134952

Merged

bors merged commit e49929e into rust-lang:master Dec 31, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 31, 2024

RalfJung deleted the arm-float-abi branch December 31, 2024 11:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

explicitly set float ABI for all ARM targets #134932

explicitly set float ABI for all ARM targets #134932

RalfJung commented Dec 30, 2024

rustbot commented Dec 30, 2024

rustbot commented Dec 30, 2024

RalfJung Dec 30, 2024

workingjubilee Dec 30, 2024

RalfJung Dec 30, 2024

RalfJung Dec 30, 2024 •

edited

Loading

madsmtm Dec 30, 2024

Urgau Dec 30, 2024

RalfJung Dec 30, 2024

workingjubilee Dec 30, 2024

workingjubilee left a comment

workingjubilee commented Dec 30, 2024

bors commented Dec 30, 2024

explicitly set float ABI for all ARM targets #134932

explicitly set float ABI for all ARM targets #134932

Conversation

RalfJung commented Dec 30, 2024

rustbot commented Dec 30, 2024

rustbot commented Dec 30, 2024

RalfJung Dec 30, 2024

Choose a reason for hiding this comment

workingjubilee Dec 30, 2024

Choose a reason for hiding this comment

RalfJung Dec 30, 2024

Choose a reason for hiding this comment

RalfJung Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

madsmtm Dec 30, 2024

Choose a reason for hiding this comment

Urgau Dec 30, 2024

Choose a reason for hiding this comment

RalfJung Dec 30, 2024

Choose a reason for hiding this comment

workingjubilee Dec 30, 2024

Choose a reason for hiding this comment

workingjubilee left a comment

Choose a reason for hiding this comment

workingjubilee commented Dec 30, 2024

bors commented Dec 30, 2024

RalfJung Dec 30, 2024 •

edited

Loading