--- title: "Design meeting 2023-12-20: Resolve ABI issues around target-feature" date: 2023-12-20 tags: ["T-lang", "design-meeting", "minutes"] discussion: https://rust-lang.zulipchat.com/#narrow/stream/410673-t-lang.2Fmeetings/topic/Design.20meeting.202023-12-20 url: https://hackmd.io/Dnd0ZIN6RjqbRlEs2GWE5Q --- # 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 are 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 define (or import via an `extern` block) 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, emitting a compiler-time error. - 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. (Other targets do not define an ABI for this case and expect the compiler to reject such code. At least that is how the author understands what happens on ARM.) 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 should 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 SIMD 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 for by-val types 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, at least on x86). 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=pentiumpro` 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) --- # Minutes People: TC, Josh, tmandry, Urgau, scottmcm, Connor Horman, Jamie Cunliffe, eholk, RalfJ, Ray Minutes/driver: TC ## Declaring functions that take SIMD vectors without target feature Josh: > 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. I would expect *calling* such functions without the target feature to be rare (though we still need a crater run). But I would expect that there's code that *declares* such functions, and then makes *calling* them conditional on having the target feature. What wold it take for us to allow *declaration* as long as the code doesn't *call* the function? I also wouldn't expect code *calling* such functions to be non-existent, such as in an emulation layer for platforms that don't have SIMD. Again, we should check. Ralf: Such code is currently unsound. If the function is declared without the target feature but called with the target feature, caller and callee use a different ABI. Josh: As is often the case, "unsound" and "doesn't work" may be two different things... Ralf: By "unsound" I mean "will actually not work in many cases". Josh: In this case, what I'd expect is happening is that either the code is compiled with the target feature, and the function is declared and called, or the code is compiled without the target feature, and the function is declared but never called. It's not compiled without and called with. Ralf: I don't think I quite understand. Would such code not use `[cfg(...)]` on the declaration? Josh: It never had to before. It could (for instance) declare several different available `extern` functions for different target features, and only call the best one that works. Ralf: I have to say this sound like a really strange setup to me and very prone to mistakes. But I suppose we could just implicitly `cfg(false)` such functions if they use a by-val argument and we don't have the ABI for it? That sounds like a possible cause of a *lot* of a confusion though. Josh: That sounds ideal, honestly, especially if we can make it implicitly `cfg(target_feature ...)` and get the compiler to use the "that's cfg-ed out" machinery to say "you can't call that because it's only available with the target feature". Ralf: We have such machinery? I wasn't aware, I thought they just disappear. But yeah I mean we can do whatever we can do on the diagnostics side. ;) Josh: Yeah, recently. If you `#[cfg(feature = "xyz")` a function, and try to call it without that feature enabled, the compiler will tell you that you need to enable the feature. It throws it away but remembers the ident and what cfg would have let it exist. Josh: Example of the code that this mechanism could keep working, if it exists in the wild: ```rust extern "C" fn some_function(..., value: __m256) -> __m256; #[cfg(target_feature = "avx")] fn another_function() { ... some_function(..., some_m256_value) } ``` If the target feature is disabled, `some_function` is declared but *never called*, so there's no UB. If the target feature is *enabled*, `some_function` is declared and called with the same target feature set. ## Runtime feature detection TC: Related to the above, it's common in many kinds of code to do runtime feature detection, e.g. for AVX512 or even AVX256, and to call particular ASM code or even Rust code with intrinsics depending on that runtime detection. What's the situation here? Ralf: If the called function is independent of the target-feature, i.e., the expanded registers aren't in the signature, then it's fine. But otherwise, it's probably unsound today, because of how LLVM works. We should actually support this, but it would take work in LLVM. Josh: Part of what I'm suggesting is that we may need to do a crater run for this. Connor: ... Josh: It's possible people will get errors that require adding a `cfg` to the function, and conceptually we could teach the compiler to act as if that `cfg` had been added, to avoid breaking existing code. (But we should only add this complexity if such code exists in the wild.) Connor: ... Ralf: I agree with need a crater run. It's obviously breaking stuff. I'd like an example in code for what Josh, you're describing. ## softfloat/hardfloat for -none targets Josh: > 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. This implies we're going to need multiple variations of `-none` targets, one for softfloat and one for hardfloat. scottmcm: might be a good thing anyway? Things like code using floats to do fast `ilog2` would like to be able to `cfg` that off on on soft-float target, for example. Ralf: Aren't the "none" targets all softfloat? I thought that was implied. It is totally possible to generate code that uses hardfloat instructions but uses a softfloat ABI. Just LLVM doesn't really seem to support that. So we can either work around that with more hardfloat targets or try to get LLVM fixed. Josh: Ah, "softfloat ABI but you can use the hardfloat instructions if they're available" is probably what -none targets want. Ralf: Right so to be clear when I say softfloat/hardfloat above I talk about ABI only. What happens in the privacy of a function body stays in the function body. (Except if LLVM screws up inlining.) Josh: Then I don't expect this will be objectionable for -none targets. --- Josh: So for AVX512, AVX256, etc., the soft ABI support is an artifact of LLVM/GCC? RalfJ: Actually, this may be done correctly in GCC and not in LLVM. Josh: We could reverse this is the future if LLVM fixes these bugs and it's stable to use. RalfJ: That requires type system changes because then we need function pointers to track this information. Josh: But soft floats and hard floats are well defined? Connor: There's not actually a canonical document on that. RalfJ: Hopefully what LLVM and GCC do is actually consistent, though. Connor: ... RalfJ: We still want to be ABI compatible with whatever GCC and Clang do. Josh: We could potentially do the lowering ourselves to always match the correct ABI, but then we wouldn't be able to mix in C code with LLVM? I'm thinking here about the SIMD case. RalfJ: Maybe? I'm not sure whether we could make it correct. The proposal here is to not support that. RalfJ: This can have crater runs, warning periods, etc. This can have all of those things. Josh: Support for checking if that's feasible, as long as we don't break code that's *working* today. RalfJ: I'm looking for agreement that we want this in general. RalfJ: The ABI should not be able to depend on target-features because we don't track that in signatures. TC: ^^^^ This is your core proposition. tmandry: I agree with that. tmandry: Are there going to be immediate demands for new targets from the users who are broken by this? Josh: There seems to be support for doing this change, and we'll have to see if it breaks any code that works today. ## What would it take to include ABI-changing target features in function pointer types? tmandry: We could smuggle ABI info in the `extern` specified, e.g. `extern "Rust+avx" fn(i128)`. We could also support coercion from `extern "Rust+avx"` to `extern "Rust"` function pointers and vice versa with a trampoline. Same for `extern "C"`. Claim: This would allow us to soundly support calling from one `#[target_feature(+avx)]` function to another with the new ABI. tmandry: I'm trying to map out, what are the difficulties here? If I could wave a magic wand and have the compiler do this, I would. Connor: I feel like this would be a wider breaking change than the current proposal, because you'd have to change certain ABIs, particularly when using `extern(C)` directly. Especially for SIMD there's also a practical issue. The ABI is broken if people are relying on `extern(C)` here. RalfJ: But that's a bug, and we shouldn't design this around bugs. RalfJ: The moment we have subtyping going on, it's not a string anymore, and we need a lattice or something. We need a rethinking of how we deal with ABIs in fn ptrs. Josh: I think I understand tmandry's proposal, and I don't think this would be a breaking change. We would generate shims. RalfJ: That would work for direct calls but not indirect calls. We don't track enough information with the function pointers for that to work correctly. tmandry: I think there's a way forward to do what I'm proposing, but we're not going to do it right now. RalfJ: I'm looking for a path right now to get to something that's not broken. tmandry: So the current proposal is that the ABI is fully defined by the target, but you can still have functions that would not be supported by that target... RalfJ: You can have those functions if they're available locally and you have the `target-feature` annotation on the function. RalfJ: We add a new pass, post-mono, that does this check. ## Next steps TC: The consensus here is that we want to do this, but... Josh: ...we're not sure whether it's possible... tmandry: ...and in particular we need to check whether we can do the simplest thing as proposed. *Consensus*: We're generally in favor of fixing this and want to see further work in this direction. We support trying to find the simplest way to do this to start, but we're uncertain of whether the proposed simplest path is possible, and we'll need to see the results of a crater run and further analysis to determine that. (The meeting ended here.) --- ## What not to rule out for the future Josh: > - 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. I don't think we should rule this out for the future. We've already talked about being able to track calling-convention ABI (`extern "C"` vs other ABIs) in function pointers somehow, so that we can safely track which kind of function we have. We've also talked about havin this work in generics somehow, so that the `Fn` traits have a (defaulted) parameter for ABI or similar, and the monomorphization of a call will call the right ABI. ## Lowering Floats to integers on softfp targets Connor: Depending on how the target/feature combination affects llvm, it may be viable to lower floating-point types to llvm integer types and floating-point operations directly to softfp libcalls. This would have the benefit of avoiding the inverse ABI problem when hardfp features are enabled directly. I don't know what other effects this has on llvm, though. Mentioned above. ## Will we need to define a bunch more targets now? tmandry: We discussed this above.