Use a generic type to implement SIMD types in core_arch::simd and avoid unsafe fn in more tests#1987
Conversation
|
|
||
| $(#[$stability])+ | ||
| impl crate::convert::From<crate::core_arch::simd::Simd<$elem_type, $len>> for $name { | ||
| #[inline(always)] | ||
| fn from(simd: crate::core_arch::simd::Simd<$elem_type, $len>) -> Self { | ||
| unsafe { crate::mem::transmute(simd) } | ||
| } | ||
| } | ||
|
|
||
| $(#[$stability])+ | ||
| impl crate::convert::From<$name> for crate::core_arch::simd::Simd<$elem_type, $len> { | ||
| #[inline(always)] | ||
| fn from(simd: $name) -> Self { | ||
| unsafe { crate::mem::transmute(simd) } | ||
| } | ||
| } |
There was a problem hiding this comment.
This is now easy to include thanks to the generic core_arch::simd::Simd type.
| // `#[derive(Clone)]` causes ICE "Projecting into SIMD type core_arch::simd::Simd is banned by MCP#838" | ||
| impl<T: SimdElement, const N: usize> Clone for Simd<T, N> { | ||
| #[inline] | ||
| fn clone(&self) -> Self { | ||
| *self | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't know why the ICE with #[derive(Clone)] did not happen with non-generic types.
There was a problem hiding this comment.
this is normal, the derived Clone implementation tries to do the clone elementwise, projecting to the SIMD type in the process. The compiler explicitly rejects any SIMD type projection via an ICE.
The derived implementation looks like this
fn clone(&self) -> Self {
Self(self.0.clone())
^ SIMD type field projection => ICE
}edit: oh I just got the problem, why does the same ICE not occur for non-generic versions. I imagine this is due to optimizations, the compiler can eliminate the field projection for non-generic versions by doing a bitwise Copy, but for generic T, the compiler cannot ensure that Copy and Clone behave the same way. For example, the following code is perfectly valid
#[derive(Copy)]
struct Foo(u32);
impl Clone for Foo {
fn clone(&self) -> Self {
println!("Cloning");
*self
}
}although I am just guessing.
eduardosm
force-pushed
the
generic-simd-type-avoid-unsafe-tests
branch
6 times, most recently
from
e08cdf5 to
c6fa637
Compare
| test_vcombine!(test_vcombine_f16 => vcombine_f16([3_f16, 4., 5., 6.], | ||
| [13_f16, 14., 15., 16.])); | ||
| #[cfg_attr(target_arch = "arm", simd_test(enable = "neon,fp16"))] | ||
| #[cfg_attr(not(target_arch = "arm"), simd_test(enable = "neon"))] | ||
| fn test_vcombine_f16() { | ||
| let a = f16x4::from_array([3_f16, 4., 5., 6.]); | ||
| let b = f16x4::from_array([13_f16, 14., 15., 16.]); | ||
| let e = f16x8::from_array([3_f16, 4., 5., 6., 13_f16, 14., 15., 16.]); | ||
| let c = f16x8::from(vcombine_f16(a.into(), b.into())); | ||
| assert_eq!(c, e); | ||
| } |
There was a problem hiding this comment.
No longer uses the test_vcombine due to the need of specifying "fp16" depending on ARM version.
eduardosm
force-pushed
the
generic-simd-type-avoid-unsafe-tests
branch
2 times, most recently
from
f51739a to
ccfe133
Compare
| ;; | ||
| armv7-*eabihf | thumbv7-*eabihf) | ||
| export RUSTFLAGS="${RUSTFLAGS} -Ctarget-feature=+neon" | ||
| export RUSTFLAGS="${RUSTFLAGS} -Ctarget-feature=+neon,+fp16" |
There was a problem hiding this comment.
Since fp16 cannot be checked at runtime (at least is_arm_feature_detected does not support it), we enable it at compile time.
This allows the `types` macro to easily implement `From<Simd<T, N>>` and `Into<Simd<T, N>>`
eduardosm
force-pushed
the
generic-simd-type-avoid-unsafe-tests
branch
from
ccfe133 to
a290873
Compare
|
The changes LGTM, although the |
3 participants
No description provided.