repr(simd) is unsound

[gnzlbg]

The following should be discussed as part of an RFC for supporting portable vector types (repr(simd)) but the current behavior is unsound (playground):

#![feature(repr_simd)]
#![feature(target_feature)]
#![allow(non_camel_case_types)]

// Given a SIMD vector type:
#[derive(Debug)]
#[repr(simd)]
struct f32x8(f32, f32, f32, f32, 
             f32, f32, f32, f32);

// and the following two functions:

#[target_feature = "+avx"]
fn foo() -> f32x8 { f32x8(0.,1.,2.,3.,4.,5.,6.,7.) }  // f32x8 will be a 256bit vector

#[target_feature = "+sse3"]
fn bar(arg: f32x8) {  // f32x8 will be 2x128bit vectors
    println!("{:?} != f32x8(0, 1, 2, 3, 4, 5, 6, 7)", arg);
    // prints: f32x8(0, 1, 2, 3, 6, 0, 0, 0) != f32x8(0, 1, 2, 3, 4, 5, 6, 7)
}

// what are the semantics of the following when
// executing on a machine that supports AVX?
fn main() { bar(foo()); }

Basically, those two objects of type f32x8 have a different layout, so foo and bar have a different ABI / calling convention. This can be introduced without target_feature, by compiling two crates with different --target-cpus and linking them, but target_feature was used here for simplicity.

[alexcrichton]

Two thoughts I've had in the past how to fix this:

• We can forbid this in trans I think by just making it a hard error. Basically when this happens give up and say "it's still your problem to fix it"
• The compiler could automatically generate a shim to "do the right thing". I explained this a long time ago as well but the general idea is that &T always has the same ABI regardless of target_feature, and the compiler could abuse that.

Given that #[target_feature] is unsafe though we could also just add it to the list of contracts you have to uphold to call the function. In that it's just one more checkbox to check when working with the unsafe functions, ensuing that you call them with the same target_feature set if you pass SIMD arguments.

---

example of unsafety in C++

example in Rust

[parched]

It's worth noting that, when talking about LLVM features (which rust target features currently map directly to) this also affects floats (which are stable). I.e., on x86 with current safe rust if you compile one crate with --target-feature=+soft-float" and one without you have an issue. This can also be solved as Alex mentions though.

[gnzlbg]

@alexcrichton I would prefer to start with a hard error, and if we need it, add a way to opt-in to the shim generation (*).

The only thing that concerns me about the hard error, is that we will probably need to emit this during monomorphization. I think that this is not acceptable, and we should only do this if it's either temporary or there is no other way. @eddyb pointed out that currently stable rust has no monomorphization-time errors, so this solution might block stabilization.

@alexcrichton you mentioned that this would mean that SIMD types must be then banned from FFI because we don't know the calling convention of the caller. Could you elaborate on this? As I see it, FFI is already unsafe, so it would be up-to-the-user to make sure that the callee is using the appropriate calling convention.

(*) I haven't thought this through, but I imagine getting a hard error for a particular call site, and then wanting to opt-in for that particular call site only, into the shim generation. Anyways, we don't need to think this all the way through now.

[alexcrichton]

I'd personally be totally ok with a hard error, but yes I think we'd have to do it during monomorphization. It's true that we don't have many monomorphization errors today but I don't think we have absolutely 0, and I'd personally also think that we should at least get to a workable state and try it out to evaluate before possibly stabilization. I, personally again, would be fine mostly likely stabilizing with monomorphization errors.

@alexcrichton you mentioned that this would mean that SIMD types must be then banned from FFI because we don't know the calling convention of the caller. Could you elaborate on this?

Oh right yeah! Right now we've got a lint in the compiler for "this type is unsafe in FFI", and for example it lints about bare structs that are not #[repr(C)] and things like String. We'd just want to make sure the lint warned about all SIMD structures as well (that they're not accidentally considered allow-by-default).

@parched I don't believe we're considering a #[target_feature] attribute for soft-float and the #[target_feature] is a whitelisted list of attributes which today go straight to LLVM but we aren't bound to always doing so. I'd imagine this would be an excellent topic of discussion were we to stabilize such a target feature!

[eddyb]

@alexcrichton I've recently reviewed all non-ICE errors from librustc_trans and the only one that can be hit from stable is monomorphization recursion limit. Anything else would be a bug IMO.

[hanna-kruppe]

While +soft-float may not be considered a valid argument for #[target_feature], disabling SSE does affect the ABI of floats. That is, this call would require a shim as well:

pub fn bar() {
    foo(0.0);
}

#[target_feature = "-sse,-sse2"]
pub fn foo(x: f32) -> f32 {
    x + x
}

The above code actually crashes in LLVM on the playground, because that's x86_64 where SSE2 is always present. However, on a 32 bit x86 target, bar passes the argument in xmm0 while foo loads its argument from the stack.

[parched]

How about we make the target spec define the ABI regardless of what extra features are enabled or disabled by attributes or the commandline. That would avoid the need for shims and monomorphization errors. So for example on x86_64 which has 128-bit vectors by default:

• -sse would be a hard error on the commandline or as an attribute on any function that has 128bit (and float?) parameters
• 256-bit vectors would be passed on the stack regardless of whether +avx is enabled. I don't think there would be a performance issue with this, because anywhere it mattered should be inlined.
• If some functions require an ABI where 256-bit registers are used, e.g., some intrinsics probably. Then we add another ABI to explicitly tag functions with e.g., extern "vector-256". Calling one of these from a function without +avx attribute would be a hard error regardless of whether +avx was enabled for the whole crate via the commandline.

[gnzlbg]

@rkruppe Thanks for that example. Two small comments:

• Solving this particular problem is not a blocker for allowing SIMD (and friends) on stable Rust. The target_feature RFC does not allow disabling features. To introduce this problem one would need to do so at the -C target-feature level, and link two crates one with +sse and one with -sse (and this is a problem we already have, with basically any feature).

• The most immediate goal is allowing SIMD on stable Rust. Ideally, whatever solution we implement first will solve this problem, but a solution that does not fix it can still be good enough as long as it doesn't prevent us from fixing this in the future.

I personally wouldn't like to have to re-open this topic in the future when people start filling bugs due to random segfaults because some crate in the middle of their dependency graph decided that it was a good idea to add -sse in their build.rs script... So, I think that we should try hard to come up with a solution that fixes all these ABI issues once and for all.

[gnzlbg]

@parched some x86_64 targets already have 512bit vectors. What do we do when AVX3 or 4 are released with support for 1024bit vectors? Where does that path end?

Ideally, if I have an SSE dynamic library that exposes some functions on its ABI for SSE...AVX2, I would like to be able to add some new AVX3/4 functions to its interface, recompile, and produce a library that is ABI compatible with the old one, so that all my old clients can continue to work as is by linking to the new library, but newer code is able to call the new AVX3/4 functions. That is, adding those new AVX3/4 functions should not break the ABI of my dynamic library as long as my global target is still sse.

[parched]

@parched some x86_64 targets already have 512bit vectors. What do we do when AVX3 or 4 are released with support for 1024bit vectors? Where does that path end?

@gnzlbg yes 512-bit and 1024-bit vectors would have to be treated the same way but I don't believe adding more would be an issue.

Ideally, if I have an SSE dynamic library that exposes some functions on its ABI for SSE...AVX2, I would like to be able to add some new AVX3/4 functions to its interface, recompile, and produce a library that is ABI compatible with the old one, so that all my old clients can continue to work as is by linking to the new library, but newer code is able to call the new AVX3/4 functions. That is, adding those new AVX3/4 functions should not break the ABI of my dynamic library as long as my global target is still sse.

For that case you would just have to make your new functions extern "vector-1024" and users wouldn't be allowed to call it unless their caller had #[target_feature = "+avx4"] . (EDIT: To be clear, you would only have to do this if your new function had 1024-bit parameters and you wanted to force them to be passed in registers rather than the stack. If you just used the default extern "Rust" 1024-bit vectors would be passed on the stack and so code would be allowed to call this function)

[gnzlbg]

@parched I think I misunderstood your comment then.

Then we add another ABI to explicitly tag functions with e.g., extern "vector-256".

Do you think that #[target_feature = "+avx"] could do this automatically?

---

@alexcrichton @BurntSushi I've slept over this a bit, and I think the following is a common idiom that we need to enable:

#[target_feature = "sse"]
fn foo(v: f32x8) -> f32x8 {
  // f32x8 has SSE ABI here
  let u = if std::host_feature(AVX) { 
      foo_avx(v)  // mismatched ABI: hard error (argument)
      // mismatched ABI: hard error (return type)
  } else {
      /* SSE code */
  }
  /* do something with u */
  u
}

#[target_feature = "avx"]
fn foo_avx(arg: f32x8) -> f32x8 { ... }

Here we have some mismatching ABIs. I am still fine with making these mismatching ABIs hard errors as long as there is an opt-in way to make this idiom work. What do you think about using as to cast between ABIs ?:

#[target_feature = "sse"]
fn foo(v: f32x8) -> f32x8 {
  // f32x8 has SSE ABI here
  let u = if std::host_feature(AVX) { 
      // foo_avx(v) // ERROR: mismatched ABIs (2x arg and ret type)
      // foo_avx(v as f32x8) // ERROR: mismatched ABIs (1x ret type)
      foo_avx(v as f32x8) as f32x8 // OK
  } else {
      /* SSE code */
  }
  /* do something with u */
  u
}

That is, an as cast to f32x8 inserts the shims that @alexcrichton was proposing above to make this work. I thought about using as #[target_feature = "sse"] f32x8 or similar but that seemed unnecessarily verbose. I also thought about making it just work, but the shims do introduce a cost, so I think it is better to make this cost explicit.

Do you think we can extend this to make function pointers work?:

#[target_feature = "+sse"] fn foo(f32x8) -> f32x8;
static mut foo_ptr: fn(f32x8) -> f32x8 = foo;

unsafe {
  // foo_ptr = foo_avx; // ERROR: mismatched ABI
  foo_ptr = foo_avx as fn(f32x8) -> f32x8; // OK
}

// assert_eq!(foo_ptr, foo_avx); // ERROR: mismatched ABIs
assert_eq!(foo_ptr, foo_avx as fn(f32x8) -> f32x8); //  OK

I was thinking that in this case, foo_avx would need to be wrapped into a function that inserts the ABI conversion shims. I was worried that doing this would introduce other issues: foo_ptr = foo_avx; assert_eq(foo_ptr, foo_avx); // FAILS because the address of foo_ptr and the address of foo_avx, but this will be an ABI mismatch error that won't compile, and doing the cast would compare the address of the wrapped function which would return OK.

I think that pursuing this would require us to track the ABI of repr(simd) types alongside their type, that is, f32x8["avx"] != f32x8["sse"] but f32x8["sse"] == f32x8["sse3"] (because their ABIs are identical. Tracking the ABI of repr(simd) types alongside the type looks like a lot of implementation work. @eddyb Do you think that "something like this" could allow us to lift the monomorphization-time errors to type-checking?

I think that if we can lift these errors to type-checking:

• that would fix this issue, because the unsound cases become type errors
• it would allow users to opt into an extra run-time cost to gain some flexibility (passing types across ABIs)
• it would fix @rkruppe's example because -sse (and soft-float) would effectively change the types of the floats, producing type errors
• it allows making function pointers work for different ABIs (and function pointers are a common idiom for dispatching to different SIMD implementations).

Thoughts?

---

EDIT: even if we never stabilize repr(simd) and only expose some f32x8 like types in std, this is slowly turning into RFC material...

---

EDIT2: That is, this issue would be resolved by making the original example fail with a type error, and adding the as conversions (or similar) would need to be a subsequent step.