# Design meeting: resolve ABI issues around target-feature

Target features generally control which instructions codegen may use during the final step of generating assembly, but some of them also affect how arguments and return values are passed across function call boundaries.
As a consequence, code compiled with different target features is not ABI-compatible and cannot be linked with each other.
In particular, since the standard library is shipped in binary form pre-built for the default set of target features, enabling/disabling particular features means that calling standard library methods suddenly causes Undefined Behavior.
That's a soundness bug that we clearly should tackle.

(The rest of this document uses examples from x86, but the issue affects basically all architectures -- though some targets [have further guards](https://github.com/rust-lang/rust/issues/117847) that can catch at least some of the UB.)

Target features are known to affect the ABI of two families of types:
1. SIMD vector types (`__m128`, `__m128i`, ...): when the target feature corresponding to the vector width of the relevant type is not enabled, the registers of that width cannot be used any more, and so the arguments must be passed in a different way. [(rustc issue)](https://github.com/rust-lang/rust/issues/114479)
2. Float types (`f32`, `f64`): when hardware float support is disabled (i.e., for softfloat targets), float types are often passed in different registers. (E.g. on x86, float return values are usually passed via x87 registers, but obviously that is not possible when x87 is disabled.) [(rustc issue)](https://github.com/rust-lang/rust/issues/116344)

The first issue can be triggered fairly easily using `#[target_feature]` attributes on `extern "C"` functions.
The second issue can only be triggered using `-C` flags like `-Ctarget-feature`, but affects even "Rust" ABI functions.

## How we could fix this

These issues is very subtle and most programmers are not aware of it.
People tend to make the (reasonable) assumption that target
features describe the set of tools the compiler has available, but don't fundamentally affect *what* the code does.
Therefore, they really shouldn't affect whether one function can even correctly pass the arguments to another function.
For instance, the [target_feature 1.1 RFC](https://github.com/rust-lang/rfcs/blob/master/text/2396-target-feature-1.1.md) implicitly assumes that it is always okay to call a function with fewer target features from one with more target features---and yet, as this issue shows, that is not true!
The RFC was written by an expert; if even experts get this wrong, we clearly need more compiler help to avoid these problems.

I would like to propose that we should achieve the following property: **When Rust object files for the same target are linked with each other, functions with the same signature will always have the same ABI.**
Our type system already ensures that functions are called with the right signature (except when you use unsafe code to circumvent it), so this would then imply that there can never be ABI mismatches on Rust-to-Rust calls, fixing the soundness bug and restoring the intuition that target features cannot affect what code does, only how it does it.
(It is still UB to call code that uses target features that the current hardware does not have, but that's much less surprising and also pretty much unavoidable.)

Fundamentally, the issue arises from a lack of information: when we call a function with signature `fn(f32) -> f32` or `fn(__m256) -> __m256`, we simply don't know which target features were enabled in the callee, so we just "hope" that it's the same target features as what we have enabled right now.

There are multiple ways we could achieve this property, and we can choose a different approach for different ABIs and types:
- Track the target features in the function signature.
This would basically mean that function pointers now have to also list the set of ABI-relevant target features that were enabled. This would be a rather fundamental change requiring new function pointer syntax, and hard to do without breaking code, so we mention it only for completeness' sake.
- Make the ABI not depend on target features. This is what we currently do for the "Rust" ABI for SIMD types (we always pass them in memory); that's why issue 1 only affects other ABIs.
- Reject declaring/calling functions with certain argument types when certain ABI features are missing. Basically, target features would be required not only to call certain intrinsics in `std::arch`, but also to even declare functions that take/return certain types.
- Reject enabling/disabling certain ABI-changing target features.

We go into more detail on the last two points, since they are the new part of this proposal.

## Reject declaring/calling functions with certain argument types when certain ABI features are missing

This could be used to fix the first issue (SIMD types).
It is essentially a summary of [this pre-RFC](https://hackmd.io/@chorman0773/SJ1rZPWZ6), though leaving some decisions open where the RFC makes concrete proposals.

The short summary of this proposal is: when you declare (or import) a `extern "C"` function that takes an `__m256` argument by-value (i.e., not behind a pointer indirection), and the `avx` target feature is not enabled, then we emit an error saying that passing such arguments via `extern "C"` requires the `avx` target feature.
This means we have an invariant that all `__m256` arguments of `extern "C"` functions are passed via AVX registers, and there is no more lack of information about how arguments are passed.

This is done for all non-Rust ABIs, I am using using "C" as an example.
It is also done for all SIMD types; `__m256` is just an example.
For each legal size of a SIMD type, we require a corresponding target feature which ensures that the registers that hold that type are available.

When we *call* a function (either directly or via a function pointer) that takes `__m256` and we do not have the `avx` target feature enabled, we have two options:
- We reject the call.
- We generate a call that uses the AVX register. This is legal since the call is anyway UB if the `avx` target feature is not available on the current host (since we are calling a function that was definitely built with that target feature). Sadly, due to LLVM limitations, this currently requires a trampoline function: LLVM cannot represent the situation where a function without the `avx` target feature wants to call a function with the `avx` target feature using the `avx` ABI.

#### Implementation considerations and consequences

Implementing this requires a table for each target architecture that says which target feature needs to be enabled to pass SIMD arguments of any given size.

This is a breaking change for code that declares/calls `extern "C"` (and generally, non-"Rust"-ABI) functions that take/returns SIMD vectors by-value without enabling the appropriate target feature.
The hope is that this is rare.
At least on x86, such functions already anyway have a strange status: curiously, there *is* a standard ABI for them, also implemented by GCC, but LLVM implements a different ABI [(llvm issue)](https://github.com/llvm/llvm-project/issues/64706).
The only way to avoid such a breaking change is to track in the function signature, and therefore in function pointer types, whether the `avx` target feature is present.
This is considered too invasive, hence the proposal says to instead reject such code.
It is not even clear whether such code was ever intentionally supported or merely works by accident.

This means that adding a SIMD type as a field to a struct can break downstream code that assumes the struct can be passed by-values to `extern "C"` functions (even if both caller and callee are defined in Rust).
However, arguably such calls shoul already only be done with "FFI-safe" types, and so this is only semver-breaking if the library documents the type to be FFI-safe.

If we go this route we should also allow delcaring target features in `extern` blocks, since otherwise it would be impossible to import a function that uses SMD types in its signature that are not enabled globally.

#### Future possibilities

Once we have such a system, we could also re-visit the decision to pass all SIMD types in the "Rust" ABI by-reference.
This was done precisely to work around this ABI problem, but it has some unfortunate performance effects -- so maybe we want to use by-register passing instead.
With this proposal, we could treat "Rust" functions like "C" functions, safely.
However, we would then reject e.g. `__m256` arguments in regular "Rust" functions, which would be a breaking change with a larger risk of regressions than just rejecting them for `extern "C"` functions.
It would also make adding a SIMD type to a struct definitely semver-breaking, since it affects even "Rust" ABI calls.
Also, if we ever do this for closures, we have to beware of [this PR](https://github.com/rust-lang/rust/pull/111836) which assumes that `target_feature` only affects codegen, not ABI. That's currently true for closures since their ABI always passes SIMD types by-ref.
So this proposal suggests we should only pass SIMD types for "Rust" calls in registers that we require are always available for this target (e.g. SSE registers on x86-64, coupled with emitting a hard error when someone attempts to disable the `sse` feature.)

This also unblocks [target_feature 1.1](https://github.com/rust-lang/rust/issues/69098), which is currently blocked over concerns that it would remove one of the `unsafe` markers in [this example affected by the ABI issue](https://github.com/rust-lang/rust/issues/116558).
Basically that RFC assumes that calling a function with *fewer* target features than the caller is always okay, but that's not actually true until we resolve the ABI problem.

#### Open questions

**Softfloat:**
What should happen when one enables SIMD features on a softfloat target, either via `-C` or via `#[target_feature]`?
Currently we accept that an then run into [LLVM failures](https://github.com/rust-lang/rust/issues/117938).
Should we reject this in rustc already?
The combination seems unsupported by LLVM currently.

However, rejecting them makes it harder to write portable code that makes use of e.g. AVX when it is present.
So an alternative might be to declare that these features are just never available at runtime in softfloat targets, so they should be rejected in `-Ctarget-features` (since it is unsound to enable features that cannot be available at runtime and there's no point in compiling code that we know is *always* unsound to run), `is_X_feature_detected!` must always return `false` for them even if the CPU actually would support the feature, and functions with `#[target_feature = "avx"]` are known to be unreachable (so hopefully there's some way we can work around the LLVM assertion failures).

That said, there are also [explicit requests to allow enabling the use of AVX features on softfloat targets](https://github.com/rust-lang/rust/issues/117938#issuecomment-1821352888).

**Make SIMD types disappear entirely:**
[This issue](https://github.com/rust-lang/rust/issues/118249) suggests we should reject use of SIMD types entirely when the corresponding target feature is missing -- so not just in the function signature, but also in the body of the function.

## Reject enabling/disabling certain ABI-changing target features

This could be used to fix the second issue (float types).

We already have several separate "softfloat" targets for code that wants to be built without the use of FPU instructions or registers (e.g. `x86_64-unknown-none`, `aarch64-unknown-none-softfloat`).
However, currently one can also use `-Ctarget-feature` to turn any hardfloat target into a softfloat target or vice versa.
(Turning a softfloat target into a hardfloat target might even be possible via `-Ctarget-cpu`.)
This entirely circumvents the point of these separate softfloat targets, which (the author assumes) were created precisely because of the ABI compatibility issues.

Thus the proposal is to reject any attempt to cross the softfloat/hardfloat barrier via `-Ctarget-feature`/`-Ctarget-cpu`.
This requires knowing, for each target architecture, which target features affect the float ABI (e.g. on x86, that's not only `x87` but also `soft-float`), and then (a) rejecting these in `#[target_feature]`, and (b) checking that after applying all `-Ctarget-*` flags, these target features are still in the same state as the "target baseline", and emitting an error otherwise.
IOW, on `i686-unknown-linux-gnu`, we should reject `-x87` and `+soft-float`, and on `x86_64-unknown-none` we should reject `-soft-float` and `+x87`.
(In fact, `+x87,+soft-float` still leads to the softfloat ABI, so we could also make the check more clever: we'd need a function that tells us, given the set of enabled target features, whether this leads to a softfloat ABI or a hardfloat ABI. On x86, softfloat is used either when `soft-float` is present or when `x87` is absent. Then we would error if the softfloat-ness of the ABI is different before and after applying the `-Ctarget-*` flags.)

### Special case: float return values on x86-32

x86-32 has a [problem with returning float values](https://github.com/rust-lang/rust/issues/115567), and one possible solution for this problem is to use an SSE register for float return values in the "Rust" ABI.
If we do that, then the `sse` target feature also becomes required for full "hardfloat" support on x86-32, and we should reject attempts to disable it on targets where we use this ABI.

(On x86-64, using an SSE register is already the default ABI for float return values, and disabling the SSE feature leads to an error when compiling a function that returns a float value, probably from LLVM: "SSE register return with SSE disabled". We could use the softfloat/hardfloat mechanism to emit a more descriptive error from rustc instead.)

The problem with this is that e.g. `-Ctarget-cpu=pentiumpro` will also disable the SSE feature, so we have to be careful to perform this check after applying `-Ctarget-cpu` -- and we break people that use `-Ctarget-cpu=pentiumpro` with one of our `i686` targets.
Arguably, they should be using a `i586` target.

### Alternative: treat float types like SIMD

Instead of outright rejecting toggling of e.g. the `x87` target feature, we could also treat this similar to how this proposal suggests we treat SIMD types, and just reject declaring functions that take/return float values when `x87` is missing.

However, that would make "softfloat" targets into "nofloat" targets, which is not their intention.
We'd need something more clever, where on softfloat targets *enabling* hardfloat features means no float functions can be declared/called any more.

The situation for SIMD and float is a bit different: for SIMD, the type is basically unavailable when some target feature is missing, but float types are always available, just their ABI changes for softfloat vs hardfloat targets.
That's why the proposal is to also treat them differently in terms of how we resolve the ABI issue.

## Conclusion

We can achieve the property that target triple + function signature contain all the information we need to compute the function ABI.
However, to do that, we have to reject declaring functions that take/return SIMD vectors when the corresponding feature is not enabled, and we have to reject using `-Ctarget-*` flags to toggle target features that determine whether a target is "softfloat" or "hardfloat".

This does however come with some downsides:
- Checking whether a function takes a SIMD type as argument can only be done post-monomorphization, so this introduces new post-monomorphization errors.
- Adding a field of SIMD type to your struct can be a breaking change if someone passes that struct across an `extern "C"` function by-value -- or maybe we need to say explicitly that you must not do that unless the library defining the type says it is FFI-safe.
- It is a breaking change for code that currently passes SIMD types across `extern "C"` *without* having the required target feature (which however uses an accidental ABI of LLVM, that disagrees with the ABI used by GCC). There is no work-around available; the assumption in this proposal is that nobody actually needs this ABI and it exists only because C compilers have to do *something* when `__m256` gets passed and `avx` is not available.
- It is a breaking change for anyone that disables `x87` on a hardfloat target -- and anyone that *enables* this on a softfloat target, though that seems very unlikely. Available workaround: use the right kind of target, rather than starting with a target that has the "wrong" floats and using target features to tweak it.
- If we are going for using SSE registers in our ABI on i686 targets, then this is also a breaking change for `-Ctarget-cpu=pentium4` and similar invocations that would disable SSE. Those would have to use e.g. `i586-unknown-linux-gnu` instead. But arguably they should do that anyway, there's a reason we have that target. Still, this is a new difficulty for tools that want to map between autoconf target triples and Rust target triples.

## Links

The two rustc issues have already been mentioned:
- [SIMD type ABI depends on target features](https://github.com/rust-lang/rust/issues/114479)
- [float type ABI depends on target features](https://github.com/rust-lang/rust/issues/116344)

Furthermore:
- There is [a PR](https://github.com/rust-lang/rust/pull/116584) to make it impossible to disable the `x87` feature (though it does not work quite the same way as in this proposal).
- There is [a Pre-RFC](https://hackmd.io/@chorman0773/SJ1rZPWZ6) for rejecting SIMD types when the corresponding target features are missing.

There's also been a ton of discussion on Zulip:
- [about said pre-RFC](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Pre-RFC.20discussion.3A.20Forbidding.20SIMD.20types.20w.2Fo.20features)
- [about target features more broadly](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/List.20of.20stable.20target.20features.3F)
- [about why one might even want to disable target features](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/The.20usecase.20for.20disabling.20target.20features.3F) (no great usecase outside of testing came up)
- [about float ABI being a mess](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/Float.20ABI.20hell)