Skip to content

What to do about repr(C, simd)? #47103

New issue
New issue
Open
Open
What to do about repr(C, simd)?#47103
Labels
A-FFIArea: Foreign function interface (FFI)A-SIMDArea: SIMD (Single Instruction Multiple Data)A-reprArea: the `#[repr(stuff)]` attributeC-enhancementCategory: An issue proposing an enhancement or a PR with one.F-simd_ffi`#![feature(simd_ffi)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
@hanna-kruppe

Description

@hanna-kruppe
hanna-kruppe
opened on Jan 1, 2018
Contributor

Currently, repr(C, simd) warns about "incompatible representation hints", but leaving off the repr(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:

  1. Require repr(C, simd) for SIMD types used in FFI.
  2. Say 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 about repr(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., permit f32x4 but not f32x3).

cc @alexcrichton

Activity

alexcrichton

alexcrichton commented on Jan 1, 2018

@alexcrichton
alexcrichton
on Jan 1, 2018
Member

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.

hanna-kruppe

hanna-kruppe commented on Jan 1, 2018

@hanna-kruppe
hanna-kruppe
on Jan 1, 2018 · edited by hanna-kruppe
ContributorAuthor

Passing vectors as immediates has all these problems and therefore is feature gated separately (feature(simd_ffi)) but if SIMD types are not considered C-compatible, even passing pointers to them will trigger the lint, which I imagine would be very annoying.

alexcrichton

alexcrichton commented on Jan 2, 2018

@alexcrichton
alexcrichton
on Jan 2, 2018
Member

Oh sure yeah, I'd expect &mut u8x32 to work just fine, I'd only expect passing u8x32 as a bare value to "basically never work"

retep998

retep998 commented on Jan 2, 2018

@retep998
retep998
on Jan 2, 2018
Member

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?

hanna-kruppe

hanna-kruppe commented on Jan 2, 2018

@hanna-kruppe
hanna-kruppe
on Jan 2, 2018 · edited by hanna-kruppe
ContributorAuthor

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.

hanna-kruppe

hanna-kruppe commented on Jan 5, 2018

@hanna-kruppe
hanna-kruppe
on Jan 5, 2018
ContributorAuthor

So in the light of the above discussion I would propose:

  • Declare some SIMD types FFI-safe irrespective of target features, based on architecture and element type and vector length. Which ones exactly is tbd, should probably be based on what C toolchains offer on the respective platform.
  • Keep warning on repr(C, simd).
  • Keep the simd_ffi feature gate which rules out passing SIMD types as immediates (this prevents any ABI mismatches due to different target_features)
hanna-kruppe
added a commit that references this issue on Jan 7, 2018

Replace empty array hack with repr(align)

9e4a692
hanna-kruppe
mentioned this on Jan 7, 2018
pietroalbini
added
C-enhancementCategory: An issue proposing an enhancement or a PR with one.
A-FFIArea: Foreign function interface (FFI)
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
A-SIMDArea: SIMD (Single Instruction Multiple Data)
on Jan 30, 2018
gnzlbg

gnzlbg commented on Mar 28, 2018

@gnzlbg
gnzlbg
on Mar 28, 2018
Contributor

Declare some SIMD types FFI-safe irrespective of target features, based on architecture and element type and vector length. Which ones exactly is tbd, should probably be based on what C toolchains offer on the respective platform.

@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 *mut f32x8 from Rust to C or vice-versa?

4 remaining items

gnzlbg

gnzlbg commented on Mar 28, 2018

@gnzlbg
gnzlbg
on Mar 28, 2018 · edited by gnzlbg
Contributor

@rkruppe

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.)

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.

hanna-kruppe

hanna-kruppe commented on Mar 28, 2018

@hanna-kruppe
hanna-kruppe
on Mar 28, 2018
ContributorAuthor

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.

hanna-kruppe

hanna-kruppe commented on Mar 28, 2018

@hanna-kruppe
hanna-kruppe
on Mar 28, 2018
ContributorAuthor

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 char to Rust's char.

gnzlbg

gnzlbg commented on Mar 28, 2018

@gnzlbg
gnzlbg
on Mar 28, 2018 · edited by gnzlbg
Contributor

@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.

hanna-kruppe

hanna-kruppe commented on Mar 28, 2018

@hanna-kruppe
hanna-kruppe
on Mar 28, 2018
ContributorAuthor

I'm fine with considering all repr(simd) types non-ffi-safe. We probably still need to define the layout, though.

gnzlbg

gnzlbg commented on Mar 28, 2018

@gnzlbg
gnzlbg
on Mar 28, 2018
Contributor

I would be fine with allowing all repr(simd) types on FFI via pointers, I just think that linting here is hard to do correctly.

If we are calling into a C function from Rust, it is fine to assume that the C compiler supports the repr(simd) type since otherwise no C library can be generated. If the signatures between the Rust and C code do not match, we should obviously produce a good error.

If we are exposing a Rust function to C, if the C compiler does not support the repr(simd) type it is its job to emit a diagnostic because we can't know what the C toolchain does or does not support.

hanna-kruppe

hanna-kruppe commented on Mar 28, 2018

@hanna-kruppe
hanna-kruppe
on Mar 28, 2018
ContributorAuthor

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.

If the signatures between the Rust and C code do not match, we should obviously produce a good error.

??? we can't. that's the whole problem with FFI.

If we are exposing a Rust function to C, if the C compiler does not support the repr(simd) type it is its job to emit a diagnostic because we can't know what the C toolchain does or does not support.

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.

gnzlbg

gnzlbg commented on Mar 28, 2018

@gnzlbg
gnzlbg
on Mar 28, 2018
Contributor

I think we are talking past each other (as usual :P). I am not against linting per se, I am against linting against some repr(simd) types but not others, because I cannot think of any good heuristic for how to separate them. Emitting a warning when someone uses any repr(simd) type on C FFI is fine to me.

hanna-kruppe

hanna-kruppe commented on Mar 28, 2018

@hanna-kruppe
hanna-kruppe
on Mar 28, 2018
ContributorAuthor

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 repr(C) to define layout without any intent to do FFI.

gnzlbg

gnzlbg commented on Mar 28, 2018

@gnzlbg
gnzlbg
on Mar 28, 2018 · edited by gnzlbg
Contributor

We probably still need to define the layout, though.

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).

Centril
added
F-simd_ffi`#![feature(simd_ffi)]`
on Jul 28, 2019
workingjubilee

workingjubilee commented on Oct 5, 2021

@workingjubilee
workingjubilee
on Oct 5, 2021
Member

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.

__m128 can only be passed by pointer to the Windows "x64 calling convention".

But we can pass some types by register to __vectorcall on x86-64-pc-windows-msvc.

workingjubilee
mentioned this on Oct 6, 2021
workingjubilee
added
A-reprArea: the `#[repr(stuff)]` attribute
on Nov 1, 2024
RalfJung

RalfJung commented on Nov 23, 2024

@RalfJung
RalfJung
on Nov 23, 2024
Member

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.

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 repr(C, simd) should be fine then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-FFIArea: Foreign function interface (FFI)A-SIMDArea: SIMD (Single Instruction Multiple Data)A-reprArea: the `#[repr(stuff)]` attributeC-enhancementCategory: An issue proposing an enhancement or a PR with one.F-simd_ffi`#![feature(simd_ffi)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @alexcrichton@RalfJung@retep998@Centril@gnzlbg

        Issue actions
          What to do about repr(C, simd)? · Issue #47103 · rust-lang/rust