-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
What to do about repr(C, simd)? #47103
Comments
Oh I was just under the impression we want to unconditionally ban simd types from C APIs. AFAIK the ABI is highly dependent on how C and Rust are compiled (target features and whatnot) and it's very tricky to get them to match up. |
Passing vectors as immediates has all these problems and therefore is feature gated separately ( |
Oh sure yeah, I'd expect |
Doesn't the alignment of SIMD types depend on which target features are enabled? If so, isn't it bad when a pointer to a SIMD type is passed around when it was allocated with a lower alignment than expected by the receiving end? |
On the Rust side, it appears vectors are always naturally aligned (rounded up to the next power of two when needed? not that it matters for types you would see in FFI). The same seems to be true in C. And now that I've thought about it, I can't really see how it could be any other way – type alignment is a global property, while target_feature can vary by function. |
So in the light of the above discussion I would propose:
|
As a side effect, this fixes the warning about repr(C, simd) that has been reported during x86_64 windows builds since rust-lang#47111 (see also: rust-lang#47103)
…richton Replace empty array hack with repr(align) As a side effect, this fixes the warning about repr(C, simd) that has been reported during x86_64 windows builds since rust-lang#47111 (see also: rust-lang#47103) r? @alexcrichton
@rkruppe do you have a concrete example that works? That is, a platform, toolchain, and SIMD type, where passing a SIMD type by pointer to C via FFI works? Do the target-features of the Rust and C binaries cause problem when passing for example a |
I'm not aware of any ABI problems with pointers to vectors -- pointers are pointers are pointers. The only possible problem would be alignment, but that's irrespective of target features as discussed above. The only reason I propose to lint for types like |
@rkruppe I would leave the lint out, at least initially. If somebody uses these types in C FFI, whatever C compiler they are using is already going to lint these for them. |
Since when do C compilers have such lints? (Since when can they even have such lints? C has no concept of a function defintion specifically for FFI, and surely you wouldn't want to warn for functions used by another translation unit of the same program.) If we don't want to get into the business of defining which vector types are fine for FFI or not, I'd rather unconditionally consider them ffi-unsafe, i.e., always lint on the Rust side. This is closer to what @alexcrichton originally proposed (though a lint, not a hard error, since the ABI concerns are nowhere near as strong). |
But I realize now that some people might be using |
What I meant is that when one uses these types on Rust FFI, one would also need to write "some types" in the C side of things. If some types are not supported by the users' C toolchain, then the user would get a compilation or linking error on the C side of things. |
But GCC does support those types as I mentioned. Allowing any and all repr(simd) types in FFI means we're basically comitting to supporting whatever GCC does with those. |
Also the FFI lint is also useful for catching errors where you think you have matching signatures on the Rust and C sides but the Rust signature "can't be right". For example, translating C's |
@rkruppe to do this right we need to know which C toolchain the user wants to use, what support does this toolchain have for repr(simd) types, and allow those. But we can't know this. |
I'm fine with considering all repr(simd) types non-ffi-safe. We probably still need to define the layout, though. |
I would be fine with allowing all If we are calling into a C function from Rust, it is fine to assume that the C compiler supports the If we are exposing a Rust function to C, if the C compiler does not support the |
I don't understand why you'd want to give no warnings at all. If there is uncertainty about or variance in what the C toolchain provides, surely we should err on the side of caution? Otherwise, we wind up implying that we match what the C compiler does, which seems impossible to guarantee if you say that "linting here is hard to do correctly". As another example, i128 is considered ffi-unsafe even though quite a few C toolchains support it.
??? we can't. that's the whole problem with FFI.
This lint doesn't even run for Rust-defined functions currently (#19834) but if it did, again, the purpose of the lint is also to warn about bindings that are very dubious on the Rust side, and therefore likely contain a human error. |
I think we are talking past each other (as usual :P). I am not against linting per se, I am against linting against some |
OK, cool. (I still don't see how I could've possibly extracted that from your last few comments, but whatever.) That still leaves the issue of using |
A lot of people expect this to work correctly: union A { data: [f32; 4], vec: f32x4, }
let x: [f32; 4] = unsafe { A { vec: f32x4::new(1., 2., 3., 4.) }.data };
assert_eq!(x[2], 3.); So I would be fine with defining vector types to have the same layout as arrays (EDIT: preserving the alignment requirements of the vector type). |
Hm. The 128-bit SIMD types (u8x16, i16x8, f32x4, u64x2) should be passable in registers on x86_64 to the System V AMD64 ABI, as far as I can tell.
But we can pass some types by register to |
This is being tackled with #116558 -- we are only allowing calls involving those types when the right target features are present. That should ensure a consistent ABI. So... allowing |
Currently,
repr(C, simd)
warns about "incompatible representation hints", but leaving off therepr(C)
triggers the FFI lint when the SIMD type is used in FFI.Note that
#[repr(C)] #[repr(simd)]
does not warn only due to #47094. Most uses of SIMD in FFI this in the Rust source tree use two separate repr attributes, which is why they didn't trigger this warning so far.There's two ways to resolve this:
repr(C, simd)
for SIMD types used in FFI.repr(simd)
is sufficient for FFI (in principle -- there are other concerns that keep SIMD-FFI feature gated at the moment) and keep the warning aboutrepr(C, simd)
. It could optionally restricted to some simd types that are known to correspond to C types on the platform in question (e.g., permitf32x4
but notf32x3
).cc @alexcrichton
The text was updated successfully, but these errors were encountered: