Skip to content

Conversation

@overmighty
Copy link
Member

FPBits::signaling_nan() defaults to setting the MSB of the payload to 1.
The tests also used signaling NaNs with a payload of 0x123. With
float16, the 1 in 0x123 aligns to the MSB of the payload, therefore
0x123 is greater than the default payload. However, that is not the case
with more precise floating-point types.

FPBits::signaling_nan() defaults to setting the MSB of the payload to 1.
The tests also used signaling NaNs with a payload of 0x123. With
float16, the 1 in 0x123 aligns to the MSB of the payload, therefore
0x123 is greater than the default payload. However, that is not the case
with more precise floating-point types.
@overmighty overmighty requested a review from lntue July 24, 2024 12:01
@llvmbot llvmbot added the libc label Jul 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 24, 2024

@llvm/pr-subscribers-libc

Author: OverMighty (overmighty)

Changes

FPBits::signaling_nan() defaults to setting the MSB of the payload to 1.
The tests also used signaling NaNs with a payload of 0x123. With
float16, the 1 in 0x123 aligns to the MSB of the payload, therefore
0x123 is greater than the default payload. However, that is not the case
with more precise floating-point types.


Full diff: https://github.com/llvm/llvm-project/pull/100354.diff

2 Files Affected:

  • (modified) libc/test/src/math/smoke/TotalOrderMagTest.h (+12-12)
  • (modified) libc/test/src/math/smoke/TotalOrderTest.h (+12-12)
diff --git a/libc/test/src/math/smoke/TotalOrderMagTest.h b/libc/test/src/math/smoke/TotalOrderMagTest.h
index 5fe2983a0e678..3d7f24fc74d10 100644
--- a/libc/test/src/math/smoke/TotalOrderMagTest.h
+++ b/libc/test/src/math/smoke/TotalOrderMagTest.h
@@ -104,24 +104,24 @@ class TotalOrderMagTestTemplate : public LIBC_NAMESPACE::testing::FEnvSafeTest {
   }
 
   void testNaNPayloads(TotalOrderMagFunc func) {
-    T qnan_123 = FPBits::quiet_nan(Sign::POS, 0x123).get_val();
-    T neg_qnan_123 = FPBits::quiet_nan(Sign::NEG, 0x123).get_val();
-    T snan_123 = FPBits::signaling_nan(Sign::POS, 0x123).get_val();
-    T neg_snan_123 = FPBits::signaling_nan(Sign::NEG, 0x123).get_val();
+    T qnan_0x42 = FPBits::quiet_nan(Sign::POS, 0x42).get_val();
+    T neg_qnan_0x42 = FPBits::quiet_nan(Sign::NEG, 0x42).get_val();
+    T snan_0x42 = FPBits::signaling_nan(Sign::POS, 0x42).get_val();
+    T neg_snan_0x42 = FPBits::signaling_nan(Sign::NEG, 0x42).get_val();
 
     EXPECT_TRUE(funcWrapper(func, aNaN, aNaN));
     EXPECT_TRUE(funcWrapper(func, sNaN, sNaN));
-    EXPECT_TRUE(funcWrapper(func, aNaN, qnan_123));
-    EXPECT_TRUE(funcWrapper(func, sNaN, snan_123));
-    EXPECT_FALSE(funcWrapper(func, qnan_123, aNaN));
-    EXPECT_FALSE(funcWrapper(func, snan_123, sNaN));
+    EXPECT_TRUE(funcWrapper(func, aNaN, qnan_0x42));
+    EXPECT_FALSE(funcWrapper(func, sNaN, snan_0x42));
+    EXPECT_FALSE(funcWrapper(func, qnan_0x42, aNaN));
+    EXPECT_TRUE(funcWrapper(func, snan_0x42, sNaN));
 
     EXPECT_TRUE(funcWrapper(func, neg_aNaN, neg_aNaN));
     EXPECT_TRUE(funcWrapper(func, neg_sNaN, neg_sNaN));
-    EXPECT_TRUE(funcWrapper(func, neg_aNaN, neg_qnan_123));
-    EXPECT_TRUE(funcWrapper(func, neg_sNaN, neg_snan_123));
-    EXPECT_FALSE(funcWrapper(func, neg_qnan_123, neg_aNaN));
-    EXPECT_FALSE(funcWrapper(func, neg_snan_123, neg_sNaN));
+    EXPECT_TRUE(funcWrapper(func, neg_aNaN, neg_qnan_0x42));
+    EXPECT_FALSE(funcWrapper(func, neg_sNaN, neg_snan_0x42));
+    EXPECT_FALSE(funcWrapper(func, neg_qnan_0x42, neg_aNaN));
+    EXPECT_TRUE(funcWrapper(func, neg_snan_0x42, neg_sNaN));
   }
 };
 
diff --git a/libc/test/src/math/smoke/TotalOrderTest.h b/libc/test/src/math/smoke/TotalOrderTest.h
index 281b2a59f930d..4d4257d089daf 100644
--- a/libc/test/src/math/smoke/TotalOrderTest.h
+++ b/libc/test/src/math/smoke/TotalOrderTest.h
@@ -102,24 +102,24 @@ class TotalOrderTestTemplate : public LIBC_NAMESPACE::testing::FEnvSafeTest {
   }
 
   void testNaNPayloads(TotalOrderFunc func) {
-    T qnan_123 = FPBits::quiet_nan(Sign::POS, 0x123).get_val();
-    T neg_qnan_123 = FPBits::quiet_nan(Sign::NEG, 0x123).get_val();
-    T snan_123 = FPBits::signaling_nan(Sign::POS, 0x123).get_val();
-    T neg_snan_123 = FPBits::signaling_nan(Sign::NEG, 0x123).get_val();
+    T qnan_0x42 = FPBits::quiet_nan(Sign::POS, 0x42).get_val();
+    T neg_qnan_0x42 = FPBits::quiet_nan(Sign::NEG, 0x42).get_val();
+    T snan_0x42 = FPBits::signaling_nan(Sign::POS, 0x42).get_val();
+    T neg_snan_0x42 = FPBits::signaling_nan(Sign::NEG, 0x42).get_val();
 
     EXPECT_TRUE(funcWrapper(func, aNaN, aNaN));
     EXPECT_TRUE(funcWrapper(func, sNaN, sNaN));
-    EXPECT_TRUE(funcWrapper(func, aNaN, qnan_123));
-    EXPECT_TRUE(funcWrapper(func, sNaN, snan_123));
-    EXPECT_FALSE(funcWrapper(func, qnan_123, aNaN));
-    EXPECT_FALSE(funcWrapper(func, snan_123, sNaN));
+    EXPECT_TRUE(funcWrapper(func, aNaN, qnan_0x42));
+    EXPECT_FALSE(funcWrapper(func, sNaN, snan_0x42));
+    EXPECT_FALSE(funcWrapper(func, qnan_0x42, aNaN));
+    EXPECT_TRUE(funcWrapper(func, snan_0x42, sNaN));
 
     EXPECT_TRUE(funcWrapper(func, neg_aNaN, neg_aNaN));
     EXPECT_TRUE(funcWrapper(func, neg_sNaN, neg_sNaN));
-    EXPECT_FALSE(funcWrapper(func, neg_aNaN, neg_qnan_123));
-    EXPECT_FALSE(funcWrapper(func, neg_sNaN, neg_snan_123));
-    EXPECT_TRUE(funcWrapper(func, neg_qnan_123, neg_aNaN));
-    EXPECT_TRUE(funcWrapper(func, neg_snan_123, neg_sNaN));
+    EXPECT_FALSE(funcWrapper(func, neg_aNaN, neg_qnan_0x42));
+    EXPECT_TRUE(funcWrapper(func, neg_sNaN, neg_snan_0x42));
+    EXPECT_TRUE(funcWrapper(func, neg_qnan_0x42, neg_aNaN));
+    EXPECT_FALSE(funcWrapper(func, neg_snan_0x42, neg_sNaN));
   }
 };
 

@overmighty overmighty merged commit 557a7b8 into llvm:main Jul 24, 2024
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
FPBits::signaling_nan() defaults to setting the MSB of the payload to 1.
The tests also used signaling NaNs with a payload of 0x123. With
float16, the 1 in 0x123 aligns to the MSB of the payload, therefore
0x123 is greater than the default payload. However, that is not the case
with more precise floating-point types.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250676
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants