Skip to content

Commit 095e4d0

Browse files
tannergoodingtannergooding
authored
Ensure the Equals instance method for the various vector types is correct (dotnet#68691)
* Adding basic tests validating NaN.Equals(NaN) returns true for various vector types * Updating the various vector APIs to use .NET compliant equality for the Equals method * Fixing the Plane and Quaternion GetHashCode tests * Removing JIT intrinsic support for the Equals instance method * Accelerating the Equals instance method for the various vector types
1 parent 7f034cf commit 095e4d0

File tree

25 files changed

+376
-140
lines changed

25 files changed

+376
-140
lines changed

‎src/coreclr/jit/simdashwintrinsiclistarm64.h‎

Lines changed: 0 additions & 4 deletions
Large diffs are not rendered by default.

‎src/coreclr/jit/simdashwintrinsiclistxarch.h‎

Lines changed: 0 additions & 5 deletions
Large diffs are not rendered by default.

‎src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,20 @@ private void TestEqualsVector<T>() where T : struct, INumber<T>
832832
Assert.False(Vector<T>.Zero.Equals(Vector<T>.One));
833833
Assert.False(Vector<T>.Zero.Equals(new Vector<T>(Util.One<T>())));
834834
}
835+
836+
[Fact]
837+
public void VectorDoubleEqualsNaNTest()
838+
{
839+
var nan = new Vector<double>(double.NaN);
840+
Assert.True(nan.Equals(nan));
841+
}
842+
843+
[Fact]
844+
public void VectorSingleEqualsNaNTest()
845+
{
846+
var nan = new Vector<float>(float.NaN);
847+
Assert.True(nan.Equals(nan));
848+
}
835849
#endregion
836850

837851
#region System.Object Overloads

‎src/libraries/System.Numerics.Vectors/tests/Matrix3x2Tests.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -983,7 +983,7 @@ public void Matrix3x2IsIdentityTest()
983983

984984
// A test for Matrix3x2 comparison involving NaN values
985985
[Fact]
986-
public void Matrix3x2EqualsNanTest()
986+
public void Matrix3x2EqualsNaNTest()
987987
{
988988
Matrix3x2 a = new Matrix3x2(float.NaN, 0, 0, 0, 0, 0);
989989
Matrix3x2 b = new Matrix3x2(0, float.NaN, 0, 0, 0, 0);
@@ -1020,13 +1020,12 @@ public void Matrix3x2EqualsNanTest()
10201020
Assert.False(e.IsIdentity);
10211021
Assert.False(f.IsIdentity);
10221022

1023-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
1024-
Assert.False(a.Equals(a));
1025-
Assert.False(b.Equals(b));
1026-
Assert.False(c.Equals(c));
1027-
Assert.False(d.Equals(d));
1028-
Assert.False(e.Equals(e));
1029-
Assert.False(f.Equals(f));
1023+
Assert.True(a.Equals(a));
1024+
Assert.True(b.Equals(b));
1025+
Assert.True(c.Equals(c));
1026+
Assert.True(d.Equals(d));
1027+
Assert.True(e.Equals(e));
1028+
Assert.True(f.Equals(f));
10301029
}
10311030

10321031
// A test to make sure these types are blittable directly into GPU buffer memory layouts

‎src/libraries/System.Numerics.Vectors/tests/Matrix4x4Tests.cs‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,7 +2490,7 @@ public void Matrix4x4From3x2Test()
24902490

24912491
// A test for Matrix4x4 comparison involving NaN values
24922492
[Fact]
2493-
public void Matrix4x4EqualsNanTest()
2493+
public void Matrix4x4EqualsNaNTest()
24942494
{
24952495
Matrix4x4 a = new Matrix4x4(float.NaN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
24962496
Matrix4x4 b = new Matrix4x4(0, float.NaN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
@@ -2577,23 +2577,22 @@ public void Matrix4x4EqualsNanTest()
25772577
Assert.False(o.IsIdentity);
25782578
Assert.False(p.IsIdentity);
25792579

2580-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
2581-
Assert.False(a.Equals(a));
2582-
Assert.False(b.Equals(b));
2583-
Assert.False(c.Equals(c));
2584-
Assert.False(d.Equals(d));
2585-
Assert.False(e.Equals(e));
2586-
Assert.False(f.Equals(f));
2587-
Assert.False(g.Equals(g));
2588-
Assert.False(h.Equals(h));
2589-
Assert.False(i.Equals(i));
2590-
Assert.False(j.Equals(j));
2591-
Assert.False(k.Equals(k));
2592-
Assert.False(l.Equals(l));
2593-
Assert.False(m.Equals(m));
2594-
Assert.False(n.Equals(n));
2595-
Assert.False(o.Equals(o));
2596-
Assert.False(p.Equals(p));
2580+
Assert.True(a.Equals(a));
2581+
Assert.True(b.Equals(b));
2582+
Assert.True(c.Equals(c));
2583+
Assert.True(d.Equals(d));
2584+
Assert.True(e.Equals(e));
2585+
Assert.True(f.Equals(f));
2586+
Assert.True(g.Equals(g));
2587+
Assert.True(h.Equals(h));
2588+
Assert.True(i.Equals(i));
2589+
Assert.True(j.Equals(j));
2590+
Assert.True(k.Equals(k));
2591+
Assert.True(l.Equals(l));
2592+
Assert.True(m.Equals(m));
2593+
Assert.True(n.Equals(n));
2594+
Assert.True(o.Equals(o));
2595+
Assert.True(p.Equals(p));
25972596
}
25982597

25992598
// A test to make sure these types are blittable directly into GPU buffer memory layouts

‎src/libraries/System.Numerics.Vectors/tests/PlaneTests.cs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public void PlaneGetHashCodeTest()
107107
{
108108
Plane target = new Plane(1.0f, 2.0f, 3.0f, 4.0f);
109109

110-
int expected = target.Normal.GetHashCode() + target.D.GetHashCode();
110+
int expected = HashCode.Combine(target.Normal, target.D);
111111
int actual = target.GetHashCode();
112112
Assert.Equal(expected, actual);
113113
}
@@ -286,7 +286,7 @@ public void PlaneTransformTest2()
286286

287287
// A test for Plane comparison involving NaN values
288288
[Fact]
289-
public void PlaneEqualsNanTest()
289+
public void PlaneEqualsNaNTest()
290290
{
291291
Plane a = new Plane(float.NaN, 0, 0, 0);
292292
Plane b = new Plane(0, float.NaN, 0, 0);
@@ -308,11 +308,10 @@ public void PlaneEqualsNanTest()
308308
Assert.False(c.Equals(new Plane(0, 0, 0, 0)));
309309
Assert.False(d.Equals(new Plane(0, 0, 0, 0)));
310310

311-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
312-
Assert.False(a.Equals(a));
313-
Assert.False(b.Equals(b));
314-
Assert.False(c.Equals(c));
315-
Assert.False(d.Equals(d));
311+
Assert.True(a.Equals(a));
312+
Assert.True(b.Equals(b));
313+
Assert.True(c.Equals(c));
314+
Assert.True(d.Equals(d));
316315
}
317316

318317
/* Enable when size of Vector3 is correct

‎src/libraries/System.Numerics.Vectors/tests/QuaternionTests.cs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ public void QuaternionGetHashCodeTest()
633633
{
634634
Quaternion a = new Quaternion(1.0f, 2.0f, 3.0f, 4.0f);
635635

636-
int expected = unchecked(a.X.GetHashCode() + a.Y.GetHashCode() + a.Z.GetHashCode() + a.W.GetHashCode());
636+
int expected = HashCode.Combine(a.X, a.Y, a.Z, a.W);
637637
int actual = a.GetHashCode();
638638
Assert.Equal(expected, actual);
639639
}
@@ -949,7 +949,7 @@ public void QuaternionIsIdentityTest()
949949

950950
// A test for Quaternion comparison involving NaN values
951951
[Fact]
952-
public void QuaternionEqualsNanTest()
952+
public void QuaternionEqualsNaNTest()
953953
{
954954
Quaternion a = new Quaternion(float.NaN, 0, 0, 0);
955955
Quaternion b = new Quaternion(0, float.NaN, 0, 0);
@@ -976,11 +976,10 @@ public void QuaternionEqualsNanTest()
976976
Assert.False(c.IsIdentity);
977977
Assert.False(d.IsIdentity);
978978

979-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
980-
Assert.False(a.Equals(a));
981-
Assert.False(b.Equals(b));
982-
Assert.False(c.Equals(c));
983-
Assert.False(d.Equals(d));
979+
Assert.True(a.Equals(a));
980+
Assert.True(b.Equals(b));
981+
Assert.True(c.Equals(c));
982+
Assert.True(d.Equals(d));
984983
}
985984

986985
// A test to make sure these types are blittable directly into GPU buffer memory layouts

‎src/libraries/System.Numerics.Vectors/tests/Vector2Tests.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,7 @@ public void Vector2EqualsTest1()
11331133

11341134
// A test for Vector2f comparison involving NaN values
11351135
[Fact]
1136-
public void Vector2EqualsNanTest()
1136+
public void Vector2EqualsNaNTest()
11371137
{
11381138
Vector2 a = new Vector2(float.NaN, 0);
11391139
Vector2 b = new Vector2(0, float.NaN);
@@ -1147,9 +1147,8 @@ public void Vector2EqualsNanTest()
11471147
Assert.False(a.Equals(Vector2.Zero));
11481148
Assert.False(b.Equals(Vector2.Zero));
11491149

1150-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
1151-
Assert.False(a.Equals(a));
1152-
Assert.False(b.Equals(b));
1150+
Assert.True(a.Equals(a));
1151+
Assert.True(b.Equals(b));
11531152
}
11541153

11551154
// A test for Reflect (Vector2f, Vector2f)

‎src/libraries/System.Numerics.Vectors/tests/Vector3Tests.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,7 @@ public void Vector3ConstructorTest5()
12161216

12171217
// A test for Vector3f comparison involving NaN values
12181218
[Fact]
1219-
public void Vector3EqualsNanTest()
1219+
public void Vector3EqualsNaNTest()
12201220
{
12211221
Vector3 a = new Vector3(float.NaN, 0, 0);
12221222
Vector3 b = new Vector3(0, float.NaN, 0);
@@ -1234,10 +1234,9 @@ public void Vector3EqualsNanTest()
12341234
Assert.False(b.Equals(Vector3.Zero));
12351235
Assert.False(c.Equals(Vector3.Zero));
12361236

1237-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
1238-
Assert.False(a.Equals(a));
1239-
Assert.False(b.Equals(b));
1240-
Assert.False(c.Equals(c));
1237+
Assert.True(a.Equals(a));
1238+
Assert.True(b.Equals(b));
1239+
Assert.True(c.Equals(c));
12411240
}
12421241

12431242
[Fact]

‎src/libraries/System.Numerics.Vectors/tests/Vector4Tests.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@ public void Vector4ConstructorTest6()
14411441

14421442
// A test for Vector4f comparison involving NaN values
14431443
[Fact]
1444-
public void Vector4EqualsNanTest()
1444+
public void Vector4EqualsNaNTest()
14451445
{
14461446
Vector4 a = new Vector4(float.NaN, 0, 0, 0);
14471447
Vector4 b = new Vector4(0, float.NaN, 0, 0);
@@ -1463,11 +1463,10 @@ public void Vector4EqualsNanTest()
14631463
Assert.False(c.Equals(Vector4.Zero));
14641464
Assert.False(d.Equals(Vector4.Zero));
14651465

1466-
// Counterintuitive result - IEEE rules for NaN comparison are weird!
1467-
Assert.False(a.Equals(a));
1468-
Assert.False(b.Equals(b));
1469-
Assert.False(c.Equals(c));
1470-
Assert.False(d.Equals(d));
1466+
Assert.True(a.Equals(a));
1467+
Assert.True(b.Equals(b));
1468+
Assert.True(c.Equals(c));
1469+
Assert.True(d.Equals(d));
14711470
}
14721471

14731472
[Fact]

0 commit comments

Comments
 (0)