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

Figure out which target features affect float ABI #131799

Open
3 of 15 tasks
RalfJung opened this issue Oct 16, 2024 · 11 comments
Open
3 of 15 tasks

Figure out which target features affect float ABI #131799

RalfJung opened this issue Oct 16, 2024 · 11 comments
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-floating-point Area: Floating point numbers and arithmetic A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Oct 16, 2024

The context for this is #116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

In #129884 I am adding the infrastructure to have the compiler recognize this. But this infrastructure needs to be fed with information about which are the "dangerous" target features. This will have to be done for each architecture we support. (Long-term we can maybe switch this around and reject all target features except the ones that are known to be harmless, but that's a looong way off since we've just allowed arbitrary unknown LLVM features to be toggled on stable with -Ctarget-feature since forever.)

  • x86 (32bit and 64bit):
  • arm
    • uses float registers if !soft-float && fpregs (see here), so toggling either can be unsound
  • aarch64
  • powerpc
  • riscv
    • "RISC-V has a similar ABI split. -F/-D/-Q is your softfloat/nofloat, but it also comes with the Zfinx/Zdinx/Zqinx variants where floating-point values are carried in the regular registers and the floating-point register file is deleted. Your float-function-features would be +F,-Zfinx, +D,-Zdinx for riscv64gc-unknown-linux (linux does not permit finx). Although I don't think this is as much of a problem because the platform states that +F,+Zfinx is illegal?" (from here)
  • wasm
  • loongarch
  • s390x
  • sparc
  • bpf
  • csky
  • hexagon
  • mips
  • m68k
  • more?
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 16, 2024
@RalfJung
Copy link
Member Author

@rust-lang/wg-llvm @bjorn3 @chorman0773 if you happen to know anything that needs to be added to the list, please let me know, or edit the issue. :) I'm mostly focusing on float ABI here; if it turns out there's a ton of architecture-specific ABI-affecting non-float features we might want to split this up into float and non-float.

@RalfJung RalfJung changed the title Figure out which target features affect ABI Figure out which target features affect float ABI Oct 16, 2024
@lolbinarycat lolbinarycat added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-floating-point Area: Floating point numbers and arithmetic A-ABI Area: Concerning the application binary interface (ABI) A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. labels Oct 16, 2024
@chorman0773
Copy link
Contributor

sse should be included for x86-64 and x87 should not be. Floats on 64-bit x86 use xmm registers rather than the x87 stack.

@RalfJung
Copy link
Member Author

Does -x87 not also disable SSE?

@chorman0773
Copy link
Contributor

chorman0773 commented Oct 16, 2024

I would assume not, they're entirely separate instruction sets and units.
In regardless, sse still affects float abi on x86_64.

@beetrees
Copy link
Contributor

beetrees commented Oct 16, 2024

@beetrees
Copy link
Contributor

beetrees commented Oct 16, 2024

@RalfJung
Copy link
Member Author

RalfJung commented Oct 17, 2024

On x86 (32-bit and 64-bit), f16 will get passed in an integer register instead of an SSE register if the sse2 feature is disabled.

What is the default register for f16 on x86-32? I would have expected it matches f32 and f64, i.e., it uses an x87 register for returns and gets passed on the stack for arguments? Why is SSE involved at all?

@beetrees
Copy link
Contributor

Oops, I meant returned (passed only applies for x86-64): you're correct that 32-bit x86 always passes arguments on the stack. I've updated my comment. On 32-bit x86, the ABI specifies that f16 gets returned in xmm0, which is what Clang and GCC do (neither Clang nor GCC support _Float16 when SSE2 is disabled).

@RalfJung
Copy link
Member Author

On 32-bit x86, the ABI specifies that f16 gets returned in xmm0, which is what Clang and GCC do (neither Clang nor GCC support _Float16 when SSE2 is disabled).

Oh wow that's Just Great. Probably justifies a separate issue: #131819

@RalfJung
Copy link
Member Author

#130988 lists a bunch of ARM target features we don't support yet. I hope none of them besides fpregs affect ABI?

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 4, 2024
…, r=workingjubilee

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature

The context for this is rust-lang#116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

So this introduces a new concept of "forbidden" target features (on top of the existing "stable " and "unstable" categories), and makes it a hard error to (un)set such a target feature. For now, the x86 and ARM feature `soft-float` is on that list. We'll have to make some effort to collect more relevant features, and similar features from other targets, but that can happen after the basic infrastructure for this landed. (These features are being collected in rust-lang#131799.)

I've made this a warning for now to give people some time to speak up if this would break something.

MCP: rust-lang/compiler-team#780
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 5, 2024
…r=workingjubilee

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature

The context for this is rust-lang#116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

So this introduces a new concept of "forbidden" target features (on top of the existing "stable " and "unstable" categories), and makes it a hard error to (un)set such a target feature. For now, the x86 and ARM feature `soft-float` is on that list. We'll have to make some effort to collect more relevant features, and similar features from other targets, but that can happen after the basic infrastructure for this landed. (These features are being collected in rust-lang#131799.)

I've made this a warning for now to give people some time to speak up if this would break something.

MCP: rust-lang/compiler-team#780
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 5, 2024
…r=workingjubilee

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature

The context for this is rust-lang#116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

So this introduces a new concept of "forbidden" target features (on top of the existing "stable " and "unstable" categories), and makes it a hard error to (un)set such a target feature. For now, the x86 and ARM feature `soft-float` is on that list. We'll have to make some effort to collect more relevant features, and similar features from other targets, but that can happen after the basic infrastructure for this landed. (These features are being collected in rust-lang#131799.)

I've made this a warning for now to give people some time to speak up if this would break something.

MCP: rust-lang/compiler-team#780
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Nov 9, 2024
…jubilee

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature

The context for this is rust-lang/rust#116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

So this introduces a new concept of "forbidden" target features (on top of the existing "stable " and "unstable" categories), and makes it a hard error to (un)set such a target feature. For now, the x86 and ARM feature `soft-float` is on that list. We'll have to make some effort to collect more relevant features, and similar features from other targets, but that can happen after the basic infrastructure for this landed. (These features are being collected in rust-lang/rust#131799.)

I've made this a warning for now to give people some time to speak up if this would break something.

MCP: rust-lang/compiler-team#780
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 9, 2024
@taiki-e
Copy link
Member

taiki-e commented Nov 11, 2024

On s390x, the soft-float changes the way of f32/f64 being passed/returned. (f128 is unaffected. f16 is unsupported.)
On 64-bit SPARC, the soft-float changes the way of f16/f32/f64/f128 being passed/returned.

I haven't checked generated asm for 32-bit SPARC (there is no tier 2 32-bit SPARC target), but considering the calling conventions and the behavior on SPARC64, I guess f16/f32/f64 are affected by the same flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-floating-point Area: Floating point numbers and arithmetic A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants