From 259ea512be351060a04dc47356697b13567e3e17 Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Wed, 9 Sep 2020 10:48:00 -0400 Subject: [PATCH 1/3] Add vcvtq_u32_f32 and vcvtq_s32_f32 These intrinsics are implemented differently for aarch64 and arm in clang. i.e. aarch64 uses the llvm.aarch64.neon.fcvtzs.v4i32.v4f32 intrinsic. However, there didn't seem to be any advantage to using that intrinsic instead of just sharing code. --- crates/core_arch/src/arm/neon/mod.rs | 38 ++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/crates/core_arch/src/arm/neon/mod.rs b/crates/core_arch/src/arm/neon/mod.rs index c733f01f76..b900e0109d 100644 --- a/crates/core_arch/src/arm/neon/mod.rs +++ b/crates/core_arch/src/arm/neon/mod.rs @@ -1813,6 +1813,28 @@ pub unsafe fn vld1q_dup_f32(addr: *const f32) -> float32x4_t { transmute(f32x4::new(v, v, v, v)) } +/// Floating-point Convert to Signed fixed-point, rounding toward Zero (vector) +#[inline] +#[target_feature(enable = "neon")] +#[cfg_attr(target_arch = "arm", target_feature(enable = "v7"))] +#[cfg_attr(all(test, target_arch = "arm"), assert_instr("vcvt.s32.f32"))] +#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(fcvtzs))] +pub unsafe fn vcvtq_s32_f32(a: float32x4_t) -> int32x4_t { + use crate::core_arch::simd::{f32x4, i32x4}; + transmute(simd_cast::<_, i32x4>(transmute::<_, f32x4>(a))) +} + +/// Floating-point Convert to Unsigned fixed-point, rounding toward Zero (vector) +#[inline] +#[target_feature(enable = "neon")] +#[cfg_attr(target_arch = "arm", target_feature(enable = "v7"))] +#[cfg_attr(all(test, target_arch = "arm"), assert_instr("vcvt.u32.f32"))] +#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(fcvtzu))] +pub unsafe fn vcvtq_u32_f32(a: float32x4_t) -> uint32x4_t { + use crate::core_arch::simd::{f32x4, u32x4}; + transmute(simd_cast::<_, u32x4>(transmute::<_, f32x4>(a))) +} + #[cfg(test)] mod tests { use super::*; @@ -1878,6 +1900,22 @@ mod tests { assert_eq!(r, e); } + #[simd_test(enable = "neon")] + unsafe fn test_vcvtq_s32_f32() { + let e = i32x4::new(-1, 2, 3, 4); + let f = f32x4::new(-1., 2., 3., 4.); + let r: i32x4 = transmute(vcvtq_s32_f32(transmute(f))); + assert_eq!(r, e); + } + + #[simd_test(enable = "neon")] + unsafe fn test_vcvtq_u32_f32() { + let e = u32x4::new(1, 2, 3, 4); + let f = f32x4::new(1., 2., 3., 4.); + let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + } + #[simd_test(enable = "neon")] unsafe fn test_vget_lane_u8() { let v = i8x8::new(1, 2, 3, 4, 5, 6, 7, 8); From 8da557519b97453d334ec7cc5f576f158f66872d Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Thu, 10 Sep 2020 17:06:56 -0400 Subject: [PATCH 2/3] Expand the test cases --- crates/core_arch/src/arm/neon/mod.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/core_arch/src/arm/neon/mod.rs b/crates/core_arch/src/arm/neon/mod.rs index b900e0109d..5a916035f5 100644 --- a/crates/core_arch/src/arm/neon/mod.rs +++ b/crates/core_arch/src/arm/neon/mod.rs @@ -1902,16 +1902,36 @@ mod tests { #[simd_test(enable = "neon")] unsafe fn test_vcvtq_s32_f32() { - let e = i32x4::new(-1, 2, 3, 4); let f = f32x4::new(-1., 2., 3., 4.); + let e = i32x4::new(-1, 2, 3, 4); let r: i32x4 = transmute(vcvtq_s32_f32(transmute(f))); assert_eq!(r, e); + + let f = f32x4::new(10e37, 2., 3., 4.); + let e = i32x4::new(0x7fffffff, 2, 3, 4); + let r: i32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(-10e37, 2., 3., 4.); + let e = i32x4::new(-0x80000000, 2, 3, 4); + let r: i32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); } #[simd_test(enable = "neon")] unsafe fn test_vcvtq_u32_f32() { - let e = u32x4::new(1, 2, 3, 4); let f = f32x4::new(1., 2., 3., 4.); + let e = u32x4::new(1, 2, 3, 4); + let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(-1., 2., 3., 4.); + let e = u32x4::new(0, 2, 3, 4); + let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(10e37, 2., 3., 4.); + let e = u32x4::new(0xffffffff, 2, 3, 4); let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); assert_eq!(r, e); } From 2ccf731dfb5bebb85f1497fed66401bf3affa95a Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Fri, 11 Sep 2020 20:03:42 -0400 Subject: [PATCH 3/3] Split out a separate implementation for aarch64 The ARM implementation uses fptoi that has undefined behaviour for out of range data. Clang has the same problem: https://llvm.org/PR47510 --- crates/core_arch/src/aarch64/neon/mod.rs | 56 ++++++++++++++++++++++++ crates/core_arch/src/arm/neon/mod.rs | 37 +++++----------- 2 files changed, 67 insertions(+), 26 deletions(-) diff --git a/crates/core_arch/src/aarch64/neon/mod.rs b/crates/core_arch/src/aarch64/neon/mod.rs index cbf3d3638a..438b1ac771 100644 --- a/crates/core_arch/src/aarch64/neon/mod.rs +++ b/crates/core_arch/src/aarch64/neon/mod.rs @@ -285,6 +285,11 @@ extern "C" { b3: int8x16_t, c: uint8x16_t, ) -> int8x16_t; + + #[link_name = "llvm.aarch64.neon.fcvtzu.v4i32.v4f32"] + fn vcvtq_u32_f32_(a: float32x4_t) -> uint32x4_t; + #[link_name = "llvm.aarch64.neon.fcvtzs.v4i32.v4f32"] + fn vcvtq_s32_f32_(a: float32x4_t) -> int32x4_t; } /// Absolute Value (wrapping). @@ -1838,6 +1843,21 @@ pub unsafe fn vld1q_u32(addr: *const u32) -> uint32x4_t { )) } +#[inline] +#[target_feature(enable = "neon")] +#[cfg_attr(test, assert_instr(fcvtzs))] +pub unsafe fn vcvtq_s32_f32(a: float32x4_t) -> int32x4_t { + vcvtq_s32_f32_(a) +} + +/// Floating-point Convert to Unsigned fixed-point, rounding toward Zero (vector) +#[inline] +#[target_feature(enable = "neon")] +#[cfg_attr(test, assert_instr(fcvtzu))] +pub unsafe fn vcvtq_u32_f32(a: float32x4_t) -> uint32x4_t { + vcvtq_u32_f32_(a) +} + #[cfg(test)] mod tests { use crate::core_arch::aarch64::test_support::*; @@ -1846,6 +1866,42 @@ mod tests { use std::mem::transmute; use stdarch_test::simd_test; + #[simd_test(enable = "neon")] + unsafe fn test_vcvtq_s32_f32() { + let f = f32x4::new(-1., 2., 3., 4.); + let e = i32x4::new(-1, 2, 3, 4); + let r: i32x4 = transmute(vcvtq_s32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(10e37, 2., 3., 4.); + let e = i32x4::new(0x7fffffff, 2, 3, 4); + let r: i32x4 = transmute(vcvtq_s32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(-10e37, 2., 3., 4.); + let e = i32x4::new(-0x80000000, 2, 3, 4); + let r: i32x4 = transmute(vcvtq_s32_f32(transmute(f))); + assert_eq!(r, e); + } + + #[simd_test(enable = "neon")] + unsafe fn test_vcvtq_u32_f32() { + let f = f32x4::new(1., 2., 3., 4.); + let e = u32x4::new(1, 2, 3, 4); + let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(-1., 2., 3., 4.); + let e = u32x4::new(0, 2, 3, 4); + let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + + let f = f32x4::new(10e37, 2., 3., 4.); + let e = u32x4::new(0xffffffff, 2, 3, 4); + let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); + assert_eq!(r, e); + } + #[simd_test(enable = "neon")] unsafe fn test_vld1q_f32() { let e = f32x4::new(1., 2., 3., 4.); diff --git a/crates/core_arch/src/arm/neon/mod.rs b/crates/core_arch/src/arm/neon/mod.rs index 5a916035f5..f1c8bb9fb6 100644 --- a/crates/core_arch/src/arm/neon/mod.rs +++ b/crates/core_arch/src/arm/neon/mod.rs @@ -1813,12 +1813,15 @@ pub unsafe fn vld1q_dup_f32(addr: *const f32) -> float32x4_t { transmute(f32x4::new(v, v, v, v)) } +// These float-to-int implementations have undefined behaviour when `a` overflows +// the destination type. Clang has the same problem: https://llvm.org/PR47510 + /// Floating-point Convert to Signed fixed-point, rounding toward Zero (vector) #[inline] +#[cfg(target_arch = "arm")] #[target_feature(enable = "neon")] -#[cfg_attr(target_arch = "arm", target_feature(enable = "v7"))] -#[cfg_attr(all(test, target_arch = "arm"), assert_instr("vcvt.s32.f32"))] -#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(fcvtzs))] +#[target_feature(enable = "v7")] +#[cfg_attr(test, assert_instr("vcvt.s32.f32"))] pub unsafe fn vcvtq_s32_f32(a: float32x4_t) -> int32x4_t { use crate::core_arch::simd::{f32x4, i32x4}; transmute(simd_cast::<_, i32x4>(transmute::<_, f32x4>(a))) @@ -1826,10 +1829,10 @@ pub unsafe fn vcvtq_s32_f32(a: float32x4_t) -> int32x4_t { /// Floating-point Convert to Unsigned fixed-point, rounding toward Zero (vector) #[inline] +#[cfg(target_arch = "arm")] #[target_feature(enable = "neon")] -#[cfg_attr(target_arch = "arm", target_feature(enable = "v7"))] -#[cfg_attr(all(test, target_arch = "arm"), assert_instr("vcvt.u32.f32"))] -#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(fcvtzu))] +#[target_feature(enable = "v7")] +#[cfg_attr(test, assert_instr("vcvt.u32.f32"))] pub unsafe fn vcvtq_u32_f32(a: float32x4_t) -> uint32x4_t { use crate::core_arch::simd::{f32x4, u32x4}; transmute(simd_cast::<_, u32x4>(transmute::<_, f32x4>(a))) @@ -1900,40 +1903,22 @@ mod tests { assert_eq!(r, e); } + #[cfg(target_arch = "arm")] #[simd_test(enable = "neon")] unsafe fn test_vcvtq_s32_f32() { let f = f32x4::new(-1., 2., 3., 4.); let e = i32x4::new(-1, 2, 3, 4); let r: i32x4 = transmute(vcvtq_s32_f32(transmute(f))); assert_eq!(r, e); - - let f = f32x4::new(10e37, 2., 3., 4.); - let e = i32x4::new(0x7fffffff, 2, 3, 4); - let r: i32x4 = transmute(vcvtq_u32_f32(transmute(f))); - assert_eq!(r, e); - - let f = f32x4::new(-10e37, 2., 3., 4.); - let e = i32x4::new(-0x80000000, 2, 3, 4); - let r: i32x4 = transmute(vcvtq_u32_f32(transmute(f))); - assert_eq!(r, e); } + #[cfg(target_arch = "arm")] #[simd_test(enable = "neon")] unsafe fn test_vcvtq_u32_f32() { let f = f32x4::new(1., 2., 3., 4.); let e = u32x4::new(1, 2, 3, 4); let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); assert_eq!(r, e); - - let f = f32x4::new(-1., 2., 3., 4.); - let e = u32x4::new(0, 2, 3, 4); - let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); - assert_eq!(r, e); - - let f = f32x4::new(10e37, 2., 3., 4.); - let e = u32x4::new(0xffffffff, 2, 3, 4); - let r: u32x4 = transmute(vcvtq_u32_f32(transmute(f))); - assert_eq!(r, e); } #[simd_test(enable = "neon")]