rust-lang / packed_simd

Portable Packed SIMD Vectors for Rust standard library
https://rust-lang.github.io/packed_simd/packed_simd_2/
Apache License 2.0
589 stars 74 forks source link

`Simd<A>` has the same layout and ABI as `A` #332

Open joshlf opened 3 years ago

joshlf commented 3 years ago

This will allow code to depend on Simd's layout. My specific use case is to emit impls of FromBytes and AsBytes for zerocopy so that other crates can safely transmute Simd instances.

workingjubilee commented 2 years ago

This patch is incorrect.

Simd<A> is at risk of having different alignment (a property of layout) or being handled differently in parameter passing (a property of ABI). We can only promise that, on the stack, Simd<A> is safe to convert to A, but that doesn't change the alignment difference, so it is not safe to reverse that. It has also been a while since I have looked at how this crate handles masks, so I feel reluctant to promise anything regarding that.

joshlf commented 2 years ago

Ah understood. Would it be sound to say that Simd has the same layout as A with the exception of alignment?

On Sun, Oct 3, 2021 at 6:11 PM Jubilee @.***> wrote:

This patch is incorrect. Simd is at risk of having different alignment or being handled differently in parameter passing. We can only promise that, on the stack, Simd is safe to convert to A, but that doesn't change the alignment difference, so it is not safe to reverse that. It has also been a while since I have looked at how this crate handles masks, so I feel reluctant to promise anything regarding that.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-lang/packed_simd/pull/332#issuecomment-933041643, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH7ML54OQIH737FREP7CHTUFDPKJANCNFSM5D6SZYPA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

workingjubilee commented 2 years ago

Hmm. This kind of thing is why #[repr(transparent)] needs to be specially marked: a struct with one field, versus a value with a type matching that field, may be passed and given different positions on the stack based on ABI details...

Can you elaborate a little more on the details of this use-case so I know exactly what sort of transformation, where, and when? I would like to know more exactly what I am promising. I looked at the crate but did not fully understand all the conversions you were offering.

workingjubilee commented 2 years ago

By the time you read this message, the Simd<T, N> and Mask<T, N> types will hopefully be available in std::simd, with #![feature(portable_simd)]. We took a different route with that module: our Mask is an opaque type. By separating the concern of masks out, we are prepared to make stronger promises about the repr of Simd, as a result, such that Simd<T, N> should be equivalent to [T; N] with a more... variable alignment... the standard caveats re: parameter passing still apply, thus it is not "exactly equivalent in ABI", but in-memory it should be consistent, as far as we are aware. This is still technically what one might consider an "implementation detail" and thus subject to change, but we are... aspiring to this being the case.