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

x86-32 "f16" ABI needs SSE, incompatible with i586 targets #131819

Open
RalfJung opened this issue Oct 17, 2024 · 8 comments
Open

x86-32 "f16" ABI needs SSE, incompatible with i586 targets #131819

RalfJung opened this issue Oct 17, 2024 · 8 comments
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged.

Comments

@RalfJung
Copy link
Member

RalfJung commented Oct 17, 2024

@beetrees wrote

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)

So... for the "Rust" ABI we can do our own thing bypassing this (likely similar fo f32/f64, we should just always return f16 in via PassMode::Indirect), but what shall we do for the extern "C" ABI? We'll need a new dedicated compiler error (likely as part of the pass that we are adding here) that rejects returning f16 when the SSE registers are not available.

That will mean that libcore does not compile on i586 targets.... Actually libcore uses the "Rust" ABI so that one is fine. But if we have extern "C" float functions somewhere those would not be portable any more.

Cc @tgross35 @chorman0773

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 17, 2024
@RalfJung RalfJung changed the title x86-32 "f16" ABI needs SSE x86-32 "f16" ABI needs SSE, incompatible with i586 targets Oct 17, 2024
@beetrees
Copy link
Contributor

I think this will require LLVM changes to properly fix, as f16 is used as the return value of builtins; this is especially apparent when when combined with #[target_feature(enable = "sse2")], like in this example. LLVM will use whatever target features a function has to decide which ABI to use for a builtin, meaning that the no_sse function presumes that the result of __truncdfhf2 has been returned in ax whereas the yes_sse2 function presumes that the result has been returned in xmm0.

A similar issue can occur on PowerPC targets with f128 and the vsx feature (cc #125109). Although #[target_feature(enable = "vsx")] is still unstable, this is still an issue if user code is compiled with -Ctarget-feature=+vsx but compiler-builtins is not (or the other way around: system f128 builtins are compiled with vsx enabled as Clang and GCC don't support _Float128/__float128 when vsx is disabled).

@RalfJung
Copy link
Member Author

Oh lol so LLVM inserts calls to builtins that use the wrong ABI? Yeah that sounds like an LLVM bug. Could you file an issue there? You probably are better at explaining the problem in their terms. :)

@jieyouxu jieyouxu added A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. labels Oct 17, 2024
@tgross35
Copy link
Contributor

I have been thinking that is would probably be in our advantage to just submit MRs against the ABIs whenever there are ambiguities like this. LLVM seems to usually pass f16 and f128 in vector registers if available (and large enough for f128), otherwise pass it as the same-sized int - but it would be good to start codifying this where applicable, considering we support these types on far more targets than C (unstably of course). __float128/_Float128 really isn't available outside of popular 64-bit targets.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 17, 2024 via email

@tgross35
Copy link
Contributor

tgross35 commented Oct 17, 2024

What would be the silent part? I was mostly suggesting that the ABIs should specify a concrete calling convention for primitives when their usual cc is feature-dependent (either via other registers or indirect). Which doesn't help us with the "target features change ABI" issue, but does help us with the "is our extern "C" correct" concern.

@RalfJung
Copy link
Member Author

The silent part is where using -Ctarget-feature is not expected to change the ABI. Code with arbitrary combinations of target features for the same target should generally be linkable with each other; any change in ABI should be explicitly requested via different flags. Also see #116344, #116558.

For "Rust" we can just define our own ABI in a consistent way -- we already special case f32/f64 returning on x86-32, we should just extend the logic here to the remaining float types.

For non-"Rust" ABI however, we have two options:

  • partition targets into "with SSE regs in the ABI" and "without SSE regs in the ABI", and reject using target features to cross that boundary
  • Reject compiling f16-returning extern "C" functions when the SSE regs are missing

@beetrees btw you said this is about sse2, but shouldn't sse be enough as that's the feature controlling the registers?

@beetrees
Copy link
Contributor

Oh lol so LLVM inserts calls to builtins that use the wrong ABI? Yeah that sounds like an LLVM bug. Could you file an issue there? You probably are better at explaining the problem in their terms. :)

I've filed an LLVM issue at llvm/llvm-project#112885.

@beetrees btw you said this is about sse2, but shouldn't sse be enough as that's the feature controlling the registers?

AFAIK it should be, but LLVM currently switches to returning in eax as soon as sse2 is disabled. I've filed an LLVM issue about it at llvm/llvm-project#112890.

@RalfJung
Copy link
Member Author

Reject compiling f16-returning extern "C" functions when the SSE regs are missing

I guess there is an alternative to this, which is to say that on x86 targets without SSE, f16 uses a different ABI for returns. Not sure if there is an official standard ABI for this, but LLVM implements some de-facto ABI. Then we have to reject ever enabling SSE on that target (including on a per-function level via #[target_feature]) to ensure the correct ABI is used everywhere.

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-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged.
Projects
None yet
Development

No branches or pull requests

5 participants