diff mbox series

arm64: rust: clean Rust 1.85.0 warning using softfloat target

Message ID 20250210163732.281786-1-ojeda@kernel.org (mailing list archive)
State New
Headers show
Series arm64: rust: clean Rust 1.85.0 warning using softfloat target | expand

Commit Message

Miguel Ojeda Feb. 10, 2025, 4:37 p.m. UTC
Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
[1] about disabling neon in the aarch64 hardfloat target:

    warning: target feature `neon` cannot be toggled with
             `-Ctarget-feature`: unsound on hard-float targets
             because it changes float ABI
      |
      = 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>

Thus, instead, use the softfloat target instead.

While trying it out, I found that the kernel sanitizers were not enabled
for that built-in target [2]. Upstream Rust agreed to backport
the enablement for the current beta so that it is ready for
the 1.85.0 release [3] -- thanks!

However, that still means that before Rust 1.85.0, we cannot switch
since sanitizers could be in use. Thus conditionally do so.

Cc: <stable@vger.kernel.org> # Needed in 6.12.y and 6.13.y only (Rust is pinned in older LTSs).
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Matthew Maurer <mmaurer@google.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Ralf Jung <post@ralfj.de>
Cc: Jubilee Young <workingjubilee@gmail.com>
Link: https://github.com/rust-lang/rust/pull/133417 [1]
Link: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/arm64.20neon.20.60-Ctarget-feature.60.20warning/near/495358442 [2]
Link: https://github.com/rust-lang/rust/pull/135905 [3]
Link: https://github.com/rust-lang/rust/issues/116344
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
Matthew: if you could please give this a go and confirm that it is fine
for Android (with sanitizers enabled, for 1.84.1 and 1.85.0), then a
Tested-by tag would be great. Thanks!

I would like to get this one into -rc3 if possible so that by the time
the compiler releases we are already clean.

I am sending another patch to introduce `rustc-min-version` and use it
here, but I am doing so after this one rather than before so that this
fix can be as simple as possible.

 arch/arm64/Makefile | 4 ++++
 1 file changed, 4 insertions(+)


base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
--
2.48.1

Comments

Will Deacon Feb. 11, 2025, 10:33 a.m. UTC | #1
On Mon, Feb 10, 2025 at 05:37:32PM +0100, Miguel Ojeda wrote:
> Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
> [1] about disabling neon in the aarch64 hardfloat target:
> 
>     warning: target feature `neon` cannot be toggled with
>              `-Ctarget-feature`: unsound on hard-float targets
>              because it changes float ABI
>       |
>       = 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>
> 
> Thus, instead, use the softfloat target instead.
> 
> While trying it out, I found that the kernel sanitizers were not enabled
> for that built-in target [2]. Upstream Rust agreed to backport
> the enablement for the current beta so that it is ready for
> the 1.85.0 release [3] -- thanks!
> 
> However, that still means that before Rust 1.85.0, we cannot switch
> since sanitizers could be in use. Thus conditionally do so.
> 
> Cc: <stable@vger.kernel.org> # Needed in 6.12.y and 6.13.y only (Rust is pinned in older LTSs).
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Matthew Maurer <mmaurer@google.com>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Ralf Jung <post@ralfj.de>
> Cc: Jubilee Young <workingjubilee@gmail.com>
> Link: https://github.com/rust-lang/rust/pull/133417 [1]
> Link: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/arm64.20neon.20.60-Ctarget-feature.60.20warning/near/495358442 [2]
> Link: https://github.com/rust-lang/rust/pull/135905 [3]
> Link: https://github.com/rust-lang/rust/issues/116344
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> Matthew: if you could please give this a go and confirm that it is fine
> for Android (with sanitizers enabled, for 1.84.1 and 1.85.0), then a
> Tested-by tag would be great. Thanks!

Patch looks fine to me, but I'll wait for Matthew to confirm that it
works for them. I'm also fine with adding the rustc-min-version helper
at the same time, tbh -- it's not exactly rocket science, but it would
need an Ack from Masahiro.

Will
Trevor Gross Feb. 11, 2025, 11:10 a.m. UTC | #2
On Mon, Feb 10, 2025 at 10:38 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
> [1] about disabling neon in the aarch64 hardfloat target:
>
>     warning: target feature `neon` cannot be toggled with
>              `-Ctarget-feature`: unsound on hard-float targets
>              because it changes float ABI
>       |
>       = 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>
>
> Thus, instead, use the softfloat target instead.
>
> While trying it out, I found that the kernel sanitizers were not enabled
> for that built-in target [2]. Upstream Rust agreed to backport
> the enablement for the current beta so that it is ready for
> the 1.85.0 release [3] -- thanks!
>
> However, that still means that before Rust 1.85.0, we cannot switch
> since sanitizers could be in use. Thus conditionally do so.
>
> Cc: <stable@vger.kernel.org> # Needed in 6.12.y and 6.13.y only (Rust is pinned in older LTSs).
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Matthew Maurer <mmaurer@google.com>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Ralf Jung <post@ralfj.de>
> Cc: Jubilee Young <workingjubilee@gmail.com>
> Link: https://github.com/rust-lang/rust/pull/133417 [1]
> Link: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/arm64.20neon.20.60-Ctarget-feature.60.20warning/near/495358442 [2]
> Link: https://github.com/rust-lang/rust/pull/135905 [3]
> Link: https://github.com/rust-lang/rust/issues/116344
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

This is consistent with what has been discussed for a while on the Rust side.

Reviewed-by: Trevor Gross <tmgross@umich.edu>
Matthew Maurer Feb. 12, 2025, 12:57 a.m. UTC | #3
I successfully booted this on the emulator (with some additional
patches to our system unrelated to this issue), so

Tested-by: Matthew Maurer <mmaurer@google.com>


On Tue, Feb 11, 2025 at 3:10 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> On Mon, Feb 10, 2025 at 10:38 AM Miguel Ojeda <ojeda@kernel.org> wrote:
> >
> > Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
> > [1] about disabling neon in the aarch64 hardfloat target:
> >
> >     warning: target feature `neon` cannot be toggled with
> >              `-Ctarget-feature`: unsound on hard-float targets
> >              because it changes float ABI
> >       |
> >       = 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>
> >
> > Thus, instead, use the softfloat target instead.
> >
> > While trying it out, I found that the kernel sanitizers were not enabled
> > for that built-in target [2]. Upstream Rust agreed to backport
> > the enablement for the current beta so that it is ready for
> > the 1.85.0 release [3] -- thanks!
> >
> > However, that still means that before Rust 1.85.0, we cannot switch
> > since sanitizers could be in use. Thus conditionally do so.
> >
> > Cc: <stable@vger.kernel.org> # Needed in 6.12.y and 6.13.y only (Rust is pinned in older LTSs).
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Matthew Maurer <mmaurer@google.com>
> > Cc: Alice Ryhl <aliceryhl@google.com>
> > Cc: Ralf Jung <post@ralfj.de>
> > Cc: Jubilee Young <workingjubilee@gmail.com>
> > Link: https://github.com/rust-lang/rust/pull/133417 [1]
> > Link: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/arm64.20neon.20.60-Ctarget-feature.60.20warning/near/495358442 [2]
> > Link: https://github.com/rust-lang/rust/pull/135905 [3]
> > Link: https://github.com/rust-lang/rust/issues/116344
> > Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
>
> This is consistent with what has been discussed for a while on the Rust side.
>
> Reviewed-by: Trevor Gross <tmgross@umich.edu>
Ralf Jung Feb. 12, 2025, 7:34 a.m. UTC | #4
On 11.02.25 12:10, Trevor Gross wrote:
> On Mon, Feb 10, 2025 at 10:38 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>>
>> Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
>> [1] about disabling neon in the aarch64 hardfloat target:
>>
>>      warning: target feature `neon` cannot be toggled with
>>               `-Ctarget-feature`: unsound on hard-float targets
>>               because it changes float ABI
>>        |
>>        = 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>
>>
>> Thus, instead, use the softfloat target instead.
>>
>> While trying it out, I found that the kernel sanitizers were not enabled
>> for that built-in target [2]. Upstream Rust agreed to backport
>> the enablement for the current beta so that it is ready for
>> the 1.85.0 release [3] -- thanks!
>>
>> However, that still means that before Rust 1.85.0, we cannot switch
>> since sanitizers could be in use. Thus conditionally do so.
>>
>> Cc: <stable@vger.kernel.org> # Needed in 6.12.y and 6.13.y only (Rust is pinned in older LTSs).
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Matthew Maurer <mmaurer@google.com>
>> Cc: Alice Ryhl <aliceryhl@google.com>
>> Cc: Ralf Jung <post@ralfj.de>
>> Cc: Jubilee Young <workingjubilee@gmail.com>
>> Link: https://github.com/rust-lang/rust/pull/133417 [1]
>> Link: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/arm64.20neon.20.60-Ctarget-feature.60.20warning/near/495358442 [2]
>> Link: https://github.com/rust-lang/rust/pull/135905 [3]
>> Link: https://github.com/rust-lang/rust/issues/116344
>> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> 
> This is consistent with what has been discussed for a while on the Rust side.
> 
> Reviewed-by: Trevor Gross <tmgross@umich.edu>

I don't know the kernel side of this, but from a Rust compiler perspective using 
the "-softfloat" target is definitely the right call here, at least for now 
(where none of the crypto/compression code that needs SIMD is written in Rust).

Reviewed-by: Ralf Jung <post@ralfj.de>

Kind regards,
Ralf
Alice Ryhl Feb. 12, 2025, 9:27 a.m. UTC | #5
On Mon, Feb 10, 2025 at 5:38 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
> [1] about disabling neon in the aarch64 hardfloat target:
>
>     warning: target feature `neon` cannot be toggled with
>              `-Ctarget-feature`: unsound on hard-float targets
>              because it changes float ABI
>       |
>       = 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>
>
> Thus, instead, use the softfloat target instead.
>
> While trying it out, I found that the kernel sanitizers were not enabled
> for that built-in target [2]. Upstream Rust agreed to backport
> the enablement for the current beta so that it is ready for
> the 1.85.0 release [3] -- thanks!
>
> However, that still means that before Rust 1.85.0, we cannot switch
> since sanitizers could be in use. Thus conditionally do so.
>
> Cc: <stable@vger.kernel.org> # Needed in 6.12.y and 6.13.y only (Rust is pinned in older LTSs).
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Matthew Maurer <mmaurer@google.com>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Ralf Jung <post@ralfj.de>
> Cc: Jubilee Young <workingjubilee@gmail.com>
> Link: https://github.com/rust-lang/rust/pull/133417 [1]
> Link: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/arm64.20neon.20.60-Ctarget-feature.60.20warning/near/495358442 [2]
> Link: https://github.com/rust-lang/rust/pull/135905 [3]
> Link: https://github.com/rust-lang/rust/issues/116344
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Thanks Matt for boot-testing it.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Miguel Ojeda Feb. 13, 2025, 12:22 p.m. UTC | #6
On Tue, Feb 11, 2025 at 11:33 AM Will Deacon <will@kernel.org> wrote:
>
> Patch looks fine to me, but I'll wait for Matthew to confirm that it
> works for them. I'm also fine with adding the rustc-min-version helper
> at the same time, tbh -- it's not exactly rocket science, but it would
> need an Ack from Masahiro.

Thanks Will -- happy to take it through the Rust tree for if needed
with your Ack, I have to send a couple other bits anyway for -rc3.

And, yeah, I went back and forth on whether to do the helper directly... :)

Cheers,
Miguel
Will Deacon Feb. 13, 2025, 1:59 p.m. UTC | #7
On Mon, 10 Feb 2025 17:37:32 +0100, Miguel Ojeda wrote:
> Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
> [1] about disabling neon in the aarch64 hardfloat target:
> 
>     warning: target feature `neon` cannot be toggled with
>              `-Ctarget-feature`: unsound on hard-float targets
>              because it changes float ABI
>       |
>       = 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>
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: rust: clean Rust 1.85.0 warning using softfloat target
      https://git.kernel.org/arm64/c/446a8351f160

Cheers,
Ard Biesheuvel Feb. 13, 2025, 3:32 p.m. UTC | #8
On Mon, 10 Feb 2025 at 17:38, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Starting with Rust 1.85.0 (to be released 2025-02-20), `rustc` warns
> [1] about disabling neon in the aarch64 hardfloat target:
>
>     warning: target feature `neon` cannot be toggled with
>              `-Ctarget-feature`: unsound on hard-float targets
>              because it changes float ABI
>       |
>       = 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>
>
> Thus, instead, use the softfloat target instead.
>

We have to carefully make the distinction here between codegen and ABI.

The arm64 C code in the kernel is built with -mgeneral-regs-only
because FP/SIMD registers are not preserved/restored like GPRs, and so
they must be used only in carefully controlled circumstances, i.e., in
assembler code called under kernel_neon_begin()/kernel_neon_end()
[modulo some exceptions related to NEON intrinsics]

This does not impact the ABI, which remains hard-float [this was the
only arm64 calling convention that existed until about a year ago].
Any function that takes or returns floats or doubles (or NEON
intrinsic types) is simply rejected by the compiler.

Changing this to softfloat for Rust modifies this calling convention,
i.e., it will result in floats and doubles being accepted as function
parameters and return values, but there is no code in the kernel that
actually supports/implements that. Also, it should be clarified
whether using a softfloat ABI permits the compiler to use FP/SIMD
registers in codegen. We might still need -Ctarget-feature="-neon"
here afaict.

Ideally, we'd have a target/target-feature combo that makes this more
explicit: no FP/SIMD codegen at all, without affecting the ABI,
therefore making float/double types in function prototypes illegal.
AIUI, this change does something different.
Ralf Jung Feb. 13, 2025, 3:46 p.m. UTC | #9
Hi all,

> We have to carefully make the distinction here between codegen and ABI.
> 
> The arm64 C code in the kernel is built with -mgeneral-regs-only
> because FP/SIMD registers are not preserved/restored like GPRs, and so
> they must be used only in carefully controlled circumstances, i.e., in
> assembler code called under kernel_neon_begin()/kernel_neon_end()
> [modulo some exceptions related to NEON intrinsics]
> 
> This does not impact the ABI, which remains hard-float [this was the
> only arm64 calling convention that existed until about a year ago].
> Any function that takes or returns floats or doubles (or NEON
> intrinsic types) is simply rejected by the compiler.

That's how C works. It is not how Rust works. Rust does not reject using floats 
ever. Instead, Rust offers softfloat targets where you can still use floats, but 
it won't use float registers. Obviously, that needs to use a different ABI.
As you said, aarch64 does not have an official softfloat ABI, but LLVM 
implements a de-facto softfloat ABI if you ask it to generate functions that 
take/return float types while disabling the relevant target features. (Maybe 
LLVM should just refuse to generate such code, and then Rust may have ended up 
with a different design. But now this would all be quite tricky to change.)

> Changing this to softfloat for Rust modifies this calling convention,
> i.e., it will result in floats and doubles being accepted as function
> parameters and return values, but there is no code in the kernel that
> actually supports/implements that.

As explained above, f32/f64 were already accepted as function parameters and 
return values in Rust code before this change. So this patch does not change 
anything here. (In fact, the ABI used for these functions should be exactly the 
same before and after this patch.)

> Also, it should be clarified
> whether using a softfloat ABI permits the compiler to use FP/SIMD
> registers in codegen. We might still need -Ctarget-feature="-neon"
> here afaict.

Rust's softfloat targets do not use FP/SIMD registers by default. Ideally these 
targets allow selectively using FP/SIMD registers within certain functions; for 
aarch64, this is not properly supported by LLVM and therefore Rust.

> Ideally, we'd have a target/target-feature combo that makes this more
> explicit: no FP/SIMD codegen at all, without affecting the ABI,
> therefore making float/double types in function prototypes illegal.
> AIUI, this change does something different.

Having targets without float support would be a significant departure from past 
language decisions in Rust -- that doesn't mean it's impossible, but it would 
require a non-trivial effort (starting with an RFC to lay down the motivation 
and design).

Kind regards,
Ralf
Gary Guo Feb. 13, 2025, 5:12 p.m. UTC | #10
On Thu, 13 Feb 2025 16:46:22 +0100
Ralf Jung <post@ralfj.de> wrote:

> Hi all,
> 
> > We have to carefully make the distinction here between codegen and ABI.
> > 
> > The arm64 C code in the kernel is built with -mgeneral-regs-only
> > because FP/SIMD registers are not preserved/restored like GPRs, and so
> > they must be used only in carefully controlled circumstances, i.e., in
> > assembler code called under kernel_neon_begin()/kernel_neon_end()
> > [modulo some exceptions related to NEON intrinsics]
> > 
> > This does not impact the ABI, which remains hard-float [this was the
> > only arm64 calling convention that existed until about a year ago].
> > Any function that takes or returns floats or doubles (or NEON
> > intrinsic types) is simply rejected by the compiler.  
> 
> That's how C works. It is not how Rust works. Rust does not reject using floats 
> ever. Instead, Rust offers softfloat targets where you can still use floats, but 
> it won't use float registers. Obviously, that needs to use a different ABI.

That's today's situation, although we do prefer to be able to turn off
floats completely (and also not have to compile parts of libcore that's
related to floats).

We mentioned this to the lang team a couple of times, and they do
acknolwedge there might be a need for it, although it's not something
that today's Rust can handle well (i.e. no feature disabling for
libcore).

We have also listed this in
https://github.com/Rust-for-Linux/linux/issues/2 and
https://github.com/Rust-for-Linux/linux/issues/514.

> As you said, aarch64 does not have an official softfloat ABI, but LLVM 
> implements a de-facto softfloat ABI if you ask it to generate functions that 
> take/return float types while disabling the relevant target features. (Maybe 
> LLVM should just refuse to generate such code, and then Rust may have ended up 
> with a different design. But now this would all be quite tricky to change.)
> 
> > Changing this to softfloat for Rust modifies this calling convention,
> > i.e., it will result in floats and doubles being accepted as function
> > parameters and return values, but there is no code in the kernel that
> > actually supports/implements that.  
> 
> As explained above, f32/f64 were already accepted as function parameters and 
> return values in Rust code before this change. So this patch does not change 
> anything here. (In fact, the ABI used for these functions should be exactly the 
> same before and after this patch.)
> 
> > Also, it should be clarified
> > whether using a softfloat ABI permits the compiler to use FP/SIMD
> > registers in codegen. We might still need -Ctarget-feature="-neon"
> > here afaict.  
> 
> Rust's softfloat targets do not use FP/SIMD registers by default. Ideally these 
> targets allow selectively using FP/SIMD registers within certain functions; for 
> aarch64, this is not properly supported by LLVM and therefore Rust.
> 
> > Ideally, we'd have a target/target-feature combo that makes this more
> > explicit: no FP/SIMD codegen at all, without affecting the ABI,
> > therefore making float/double types in function prototypes illegal.
> > AIUI, this change does something different.  
> 
> Having targets without float support would be a significant departure from past 
> language decisions in Rust -- that doesn't mean it's impossible, but it would 
> require a non-trivial effort (starting with an RFC to lay down the motivation 
> and design).
> 
> Kind regards,
> Ralf
> 
>
Ard Biesheuvel Feb. 13, 2025, 5:18 p.m. UTC | #11
On Thu, 13 Feb 2025 at 16:46, Ralf Jung <post@ralfj.de> wrote:
>
> Hi all,
>
> > We have to carefully make the distinction here between codegen and ABI.
> >
> > The arm64 C code in the kernel is built with -mgeneral-regs-only
> > because FP/SIMD registers are not preserved/restored like GPRs, and so
> > they must be used only in carefully controlled circumstances, i.e., in
> > assembler code called under kernel_neon_begin()/kernel_neon_end()
> > [modulo some exceptions related to NEON intrinsics]
> >
> > This does not impact the ABI, which remains hard-float [this was the
> > only arm64 calling convention that existed until about a year ago].
> > Any function that takes or returns floats or doubles (or NEON
> > intrinsic types) is simply rejected by the compiler.
>
> That's how C works. It is not how Rust works. Rust does not reject using floats
> ever. Instead, Rust offers softfloat targets where you can still use floats, but
> it won't use float registers. Obviously, that needs to use a different ABI.
> As you said, aarch64 does not have an official softfloat ABI, but LLVM
> implements a de-facto softfloat ABI if you ask it to generate functions that
> take/return float types while disabling the relevant target features. (Maybe
> LLVM should just refuse to generate such code, and then Rust may have ended up
> with a different design. But now this would all be quite tricky to change.)
>
> > Changing this to softfloat for Rust modifies this calling convention,
> > i.e., it will result in floats and doubles being accepted as function
> > parameters and return values, but there is no code in the kernel that
> > actually supports/implements that.
>
> As explained above, f32/f64 were already accepted as function parameters and
> return values in Rust code before this change. So this patch does not change
> anything here. (In fact, the ABI used for these functions should be exactly the
> same before and after this patch.)
>

OK, so can I summarize the above as

- Rust calling Rust will work fine and happily use float types without
using FP/SIMD registers in codegen;
- Rust calling C or C calling Rust will not support float or double
arguments or return values due to the restrictions imposed by the C
compiler.

?

> > Also, it should be clarified
> > whether using a softfloat ABI permits the compiler to use FP/SIMD
> > registers in codegen. We might still need -Ctarget-feature="-neon"
> > here afaict.
>
> Rust's softfloat targets do not use FP/SIMD registers by default. Ideally these
> targets allow selectively using FP/SIMD registers within certain functions; for
> aarch64, this is not properly supported by LLVM and therefore Rust.
>

I read this as 'this default behavior might change in the future', and
so -Ctarget-feature="-neon" should be added even if it is redundant at
this point in time.

> > Ideally, we'd have a target/target-feature combo that makes this more
> > explicit: no FP/SIMD codegen at all, without affecting the ABI,
> > therefore making float/double types in function prototypes illegal.
> > AIUI, this change does something different.
>
> Having targets without float support would be a significant departure from past
> language decisions in Rust -- that doesn't mean it's impossible, but it would
> require a non-trivial effort (starting with an RFC to lay down the motivation
> and design).
>

Fair enough. The codegen is all that matters, and there are other
cases (e.g., spilling) where the compiler may decide to use FP/SIMD
registers without any floats or doubles in sight. In fact, there are
swaths of non-performance critical floating point code in the AMDGPU
driver where it would be useful to have float/double support using
softfloat codegen too.
Ralf Jung Feb. 13, 2025, 8:17 p.m. UTC | #12
Hi,

>>> We have to carefully make the distinction here between codegen and ABI.
>>>
>>> The arm64 C code in the kernel is built with -mgeneral-regs-only
>>> because FP/SIMD registers are not preserved/restored like GPRs, and so
>>> they must be used only in carefully controlled circumstances, i.e., in
>>> assembler code called under kernel_neon_begin()/kernel_neon_end()
>>> [modulo some exceptions related to NEON intrinsics]
>>>
>>> This does not impact the ABI, which remains hard-float [this was the
>>> only arm64 calling convention that existed until about a year ago].
>>> Any function that takes or returns floats or doubles (or NEON
>>> intrinsic types) is simply rejected by the compiler.
>>
>> That's how C works. It is not how Rust works. Rust does not reject using floats
>> ever. Instead, Rust offers softfloat targets where you can still use floats, but
>> it won't use float registers. Obviously, that needs to use a different ABI.
>> As you said, aarch64 does not have an official softfloat ABI, but LLVM
>> implements a de-facto softfloat ABI if you ask it to generate functions that
>> take/return float types while disabling the relevant target features. (Maybe
>> LLVM should just refuse to generate such code, and then Rust may have ended up
>> with a different design. But now this would all be quite tricky to change.)
>>
>>> Changing this to softfloat for Rust modifies this calling convention,
>>> i.e., it will result in floats and doubles being accepted as function
>>> parameters and return values, but there is no code in the kernel that
>>> actually supports/implements that.
>>
>> As explained above, f32/f64 were already accepted as function parameters and
>> return values in Rust code before this change. So this patch does not change
>> anything here. (In fact, the ABI used for these functions should be exactly the
>> same before and after this patch.)
>>
> 
> OK, so can I summarize the above as
> 
> - Rust calling Rust will work fine and happily use float types without
> using FP/SIMD registers in codegen;
> - Rust calling C or C calling Rust will not support float or double
> arguments or return values due to the restrictions imposed by the C
> compiler.
> 
> ?

Correct. (I assume the existing Rust kernel code contains no float or SIMD types 
so this is largely theoretical. But I'm a Rust compiler / upstream dev, not a 
Rust-for-Linux / kernel dev, so I might be entirely off here.)

> 
>>> Also, it should be clarified
>>> whether using a softfloat ABI permits the compiler to use FP/SIMD
>>> registers in codegen. We might still need -Ctarget-feature="-neon"
>>> here afaict.
>>
>> Rust's softfloat targets do not use FP/SIMD registers by default. Ideally these
>> targets allow selectively using FP/SIMD registers within certain functions; for
>> aarch64, this is not properly supported by LLVM and therefore Rust.
>>
> 
> I read this as 'this default behavior might change in the future', and
> so -Ctarget-feature="-neon" should be added even if it is redundant at
> this point in time.

Personally I think it would be a breaking change to start using neon in the 
aarch64-softfloat target, for reasons such as what we are discussing. (And this 
means we won't do it.) We have generally, but implicitly, understood "softfloat" 
to mean both "uses softfloat ABI" and "does not use any FP/SIMD registers by 
default". But I don't know whether the Rust compiler team has formally committed 
to anything here. I can inquire about that if you want.

OTOH I cannot see how adding "-neon" could possibly hurt, either. It's juts a 
NOP currently.

Kind regards,
Ralf


> 
>>> Ideally, we'd have a target/target-feature combo that makes this more
>>> explicit: no FP/SIMD codegen at all, without affecting the ABI,
>>> therefore making float/double types in function prototypes illegal.
>>> AIUI, this change does something different.
>>
>> Having targets without float support would be a significant departure from past
>> language decisions in Rust -- that doesn't mean it's impossible, but it would
>> require a non-trivial effort (starting with an RFC to lay down the motivation
>> and design).
>>
> 
> Fair enough. The codegen is all that matters, and there are other
> cases (e.g., spilling) where the compiler may decide to use FP/SIMD
> registers without any floats or doubles in sight. In fact, there are
> swaths of non-performance critical floating point code in the AMDGPU
> driver where it would be useful to have float/double support using
> softfloat codegen too.
diff mbox series

Patch

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 358c68565bfd..2b25d671365f 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -48,7 +48,11 @@  KBUILD_CFLAGS	+= $(CC_FLAGS_NO_FPU) \
 KBUILD_CFLAGS	+= $(call cc-disable-warning, psabi)
 KBUILD_AFLAGS	+= $(compat_vdso)

+ifeq ($(call test-ge, $(CONFIG_RUSTC_VERSION), 108500),y)
+KBUILD_RUSTFLAGS += --target=aarch64-unknown-none-softfloat
+else
 KBUILD_RUSTFLAGS += --target=aarch64-unknown-none -Ctarget-feature="-neon"
+endif

 KBUILD_CFLAGS	+= $(call cc-option,-mabi=lp64)
 KBUILD_AFLAGS	+= $(call cc-option,-mabi=lp64)