Fix negation and signum to consistently handle negative zero across p…

…latforms (#344) * Fix negation and signum to consistently handle negative zero across platforms * rustfmt
bitshifter · Sep 30, 2022 · 4cc6967 · 4cc6967
1 parent 3ec3753
commit 4cc6967
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 27 deletions.
diff --git a/codegen/templates/vec.rs.tera b/codegen/templates/vec.rs.tera
@@ -811,11 +811,18 @@ impl {{ self_t }} {
             }
         {% elif is_coresimd %}
             Self(self.0.signum())
-        {% else %}
-            let mask = self.cmpge(Self::ZERO);
-            let result = Self::select(mask, Self::ONE, Self::NEG_ONE);
-            let mask = self.is_nan_mask();
-            Self::select(mask, self, result)
+        {% elif is_sse2 %}
+            unsafe {
+                let result = Self(_mm_or_ps(_mm_and_ps(self.0, Self::NEG_ONE.0), Self::ONE.0));
+                let mask = self.is_nan_mask();
+                Self::select(mask, self, result)
+            }
+        {% elif is_wasm32 %}
+            unsafe {
+                let result = Self(v128_or(v128_and(self.0, Self::NEG_ONE.0), Self::ONE.0));
+                let mask = self.is_nan_mask();
+                Self::select(mask, self, result)
+            }
         {% endif %}
     }
 {% endif %}
@@ -2035,7 +2042,7 @@ impl Neg for {{ self_t }} {
                 {%- endfor %}
             }
         {% elif is_sse2 %}
-            Self(unsafe { _mm_sub_ps(Self::ZERO.0, self.0) })
+            Self(unsafe { _mm_xor_ps(_mm_set1_ps(-0.0), self.0) })
         {% elif is_wasm32 %}
             Self(f32x4_neg(self.0))
         {% elif is_coresimd %}

diff --git a/src/f32/sse2/vec3a.rs b/src/f32/sse2/vec3a.rs
@@ -312,10 +312,11 @@ impl Vec3A {
     /// - `NAN` if the number is `NAN`
     #[inline]
     pub fn signum(self) -> Self {
-        let mask = self.cmpge(Self::ZERO);
-        let result = Self::select(mask, Self::ONE, Self::NEG_ONE);
-        let mask = self.is_nan_mask();
-        Self::select(mask, self, result)
+        unsafe {
+            let result = Self(_mm_or_ps(_mm_and_ps(self.0, Self::NEG_ONE.0), Self::ONE.0));
+            let mask = self.is_nan_mask();
+            Self::select(mask, self, result)
+        }
     }
 
     /// Returns `true` if, and only if, all elements are finite.  If any element is either
@@ -993,7 +994,7 @@ impl Neg for Vec3A {
     type Output = Self;
     #[inline]
     fn neg(self) -> Self {
-        Self(unsafe { _mm_sub_ps(Self::ZERO.0, self.0) })
+        Self(unsafe { _mm_xor_ps(_mm_set1_ps(-0.0), self.0) })
     }
 }
 

diff --git a/src/f32/sse2/vec4.rs b/src/f32/sse2/vec4.rs
@@ -287,10 +287,11 @@ impl Vec4 {
     /// - `NAN` if the number is `NAN`
     #[inline]
     pub fn signum(self) -> Self {
-        let mask = self.cmpge(Self::ZERO);
-        let result = Self::select(mask, Self::ONE, Self::NEG_ONE);
-        let mask = self.is_nan_mask();
-        Self::select(mask, self, result)
+        unsafe {
+            let result = Self(_mm_or_ps(_mm_and_ps(self.0, Self::NEG_ONE.0), Self::ONE.0));
+            let mask = self.is_nan_mask();
+            Self::select(mask, self, result)
+        }
     }
 
     /// Returns `true` if, and only if, all elements are finite.  If any element is either
@@ -906,7 +907,7 @@ impl Neg for Vec4 {
     type Output = Self;
     #[inline]
     fn neg(self) -> Self {
-        Self(unsafe { _mm_sub_ps(Self::ZERO.0, self.0) })
+        Self(unsafe { _mm_xor_ps(_mm_set1_ps(-0.0), self.0) })
     }
 }
 

diff --git a/src/f32/wasm32/vec3a.rs b/src/f32/wasm32/vec3a.rs
@@ -293,10 +293,11 @@ impl Vec3A {
     /// - `NAN` if the number is `NAN`
     #[inline]
     pub fn signum(self) -> Self {
-        let mask = self.cmpge(Self::ZERO);
-        let result = Self::select(mask, Self::ONE, Self::NEG_ONE);
-        let mask = self.is_nan_mask();
-        Self::select(mask, self, result)
+        unsafe {
+            let result = Self(v128_or(v128_and(self.0, Self::NEG_ONE.0), Self::ONE.0));
+            let mask = self.is_nan_mask();
+            Self::select(mask, self, result)
+        }
     }
 
     /// Returns `true` if, and only if, all elements are finite.  If any element is either

diff --git a/src/f32/wasm32/vec4.rs b/src/f32/wasm32/vec4.rs
@@ -275,10 +275,11 @@ impl Vec4 {
     /// - `NAN` if the number is `NAN`
     #[inline]
     pub fn signum(self) -> Self {
-        let mask = self.cmpge(Self::ZERO);
-        let result = Self::select(mask, Self::ONE, Self::NEG_ONE);
-        let mask = self.is_nan_mask();
-        Self::select(mask, self, result)
+        unsafe {
+            let result = Self(v128_or(v128_and(self.0, Self::NEG_ONE.0), Self::ONE.0));
+            let mask = self.is_nan_mask();
+            Self::select(mask, self, result)
+        }
     }
 
     /// Returns `true` if, and only if, all elements are finite.  If any element is either

diff --git a/tests/vec2.rs b/tests/vec2.rs
@@ -478,6 +478,8 @@ macro_rules! impl_vec2_signed_tests {
         glam_test!(test_neg, {
             let a = $new(1 as $t, 2 as $t);
             assert_eq!($new(-1 as $t, -2 as $t), (-a));
+            assert_eq!($new(-0.0 as $t, -0.0 as $t), -$new(0.0 as $t, 0.0 as $t));
+            assert_eq!($new(0.0 as $t, -0.0 as $t), -$new(-0.0 as $t, 0.0 as $t));
         });
 
         glam_test!(test_perp, {
@@ -596,7 +598,7 @@ macro_rules! impl_vec2_float_tests {
 
         glam_test!(test_sign, {
             assert_eq!($vec2::ZERO.signum(), $vec2::ONE);
-            assert_eq!(-$vec2::ZERO.signum(), -$vec2::ONE);
+            assert_eq!((-$vec2::ZERO).signum(), -$vec2::ONE);
             assert_eq!($vec2::ONE.signum(), $vec2::ONE);
             assert_eq!((-$vec2::ONE).signum(), -$vec2::ONE);
             assert_eq!($vec2::splat(INFINITY).signum(), $vec2::ONE);

diff --git a/tests/vec3.rs b/tests/vec3.rs
@@ -523,6 +523,14 @@ macro_rules! impl_vec3_signed_tests {
         glam_test!(test_neg, {
             let a = $new(1 as $t, 2 as $t, 3 as $t);
             assert_eq!((-1 as $t, -2 as $t, -3 as $t), (-a).into());
+            assert_eq!(
+                $new(-0.0 as $t, -0.0 as $t, -0.0 as $t),
+                -$new(0.0 as $t, 0.0 as $t, 0.0 as $t)
+            );
+            assert_eq!(
+                $new(0.0 as $t, -0.0 as $t, -0.0 as $t),
+                -$new(-0.0 as $t, 0.0 as $t, 0.0 as $t)
+            );
         });
 
         glam_test!(test_dot_signed, {
@@ -652,7 +660,7 @@ macro_rules! impl_vec3_float_tests {
 
         glam_test!(test_signum, {
             assert_eq!($vec3::ZERO.signum(), $vec3::ONE);
-            assert_eq!(-$vec3::ZERO.signum(), -$vec3::ONE);
+            assert_eq!((-$vec3::ZERO).signum(), -$vec3::ONE);
             assert_eq!($vec3::ONE.signum(), $vec3::ONE);
             assert_eq!((-$vec3::ONE).signum(), -$vec3::ONE);
             assert_eq!($vec3::splat(INFINITY).signum(), $vec3::ONE);

diff --git a/tests/vec4.rs b/tests/vec4.rs
@@ -598,6 +598,14 @@ macro_rules! impl_vec4_signed_tests {
         glam_test!(test_neg, {
             let a = $new(1 as $t, 2 as $t, 3 as $t, 4 as $t);
             assert_eq!((-1 as $t, -2 as $t, -3 as $t, -4 as $t), (-a).into());
+            assert_eq!(
+                $new(-0.0 as $t, -0.0 as $t, -0.0 as $t, -0.0 as $t),
+                -$new(0.0 as $t, 0.0 as $t, 0.0 as $t, 0.0 as $t)
+            );
+            assert_eq!(
+                $new(0.0 as $t, -0.0 as $t, -0.0 as $t, -0.0 as $t),
+                -$new(-0.0 as $t, 0.0 as $t, 0.0 as $t, 0.0 as $t)
+            );
         });
 
         glam_test!(test_dot_signed, {
@@ -734,7 +742,7 @@ macro_rules! impl_vec4_float_tests {
 
         glam_test!(test_signum, {
             assert_eq!($vec4::ZERO.signum(), $vec4::ONE);
-            assert_eq!(-$vec4::ZERO.signum(), -$vec4::ONE);
+            assert_eq!((-$vec4::ZERO).signum(), -$vec4::ONE);
             assert_eq!($vec4::ONE.signum(), $vec4::ONE);
             assert_eq!((-$vec4::ONE).signum(), -$vec4::ONE);
             assert_eq!($vec4::splat(INFINITY).signum(), $vec4::ONE);