Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong test data in C verification #241

Closed
asmfreak opened this issue Feb 24, 2022 · 1 comment
Closed

Wrong test data in C verification #241

asmfreak opened this issue Feb 24, 2022 · 1 comment
Labels
bug Something isn't working build and test trivial

Comments

@asmfreak
Copy link
Contributor

Test data on float point conversion is wrong. In the test data for float16 packing for C language the mask for the sign bit is 0x80000, but it should be 0x8000. Here is a patch fixing the problem:

diff --git a/verification/c/suite/test_support.c b/verification/c/suite/test_support.c
index 8b79153..feaf76b 100644
--- a/verification/c/suite/test_support.c
+++ b/verification/c/suite/test_support.c
@@ -424,37 +424,37 @@ static void testNunavutFloat16Pack_NAN_cmath(void)
 {
     uint16_t packed_float = nunavutFloat16Pack(NAN);
     TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x7C00, (0x7C00UL & packed_float), "Exponent bits were not all set for NAN.");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x80000UL & packed_float), "NAN sign bit was negative.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x8000UL & packed_float), "NAN sign bit was negative.");
 
     packed_float = nunavutFloat16Pack(-NAN);
     TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x7C00, (0x7C00UL & packed_float), "Exponent bits were not all set for -NAN.");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x80000, (0x80000UL & packed_float), "-NAN sign bit was positive.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x8000, (0x8000UL & packed_float), "-NAN sign bit was positive.");
 }
 
 static void testNunavutFloat16Pack_infinity(void)
 {
     uint16_t packed_float = nunavutFloat16Pack(INFINITY);
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x3FF & packed_float), "Mantessa bits were not 0 for INFINITY.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x03FF & packed_float), "Mantessa bits were not 0 for INFINITY.");
     TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x7C00, (0x7C00 & packed_float), "INFINITY did not set bits G5 - G4+w");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x80000 & packed_float), "INFINITY sign bit was negative.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x8000 & packed_float), "INFINITY sign bit was negative.");
 
     packed_float = nunavutFloat16Pack(-INFINITY);
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x3FF & packed_float), "Mantessa bits were not 0 for -INFINITY.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x03FF & packed_float), "Mantessa bits were not 0 for -INFINITY.");
     TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x7C00, (0x7C00 & packed_float), "-INFINITY did not set bits G5 - G4+w");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x80000, (0x80000 & packed_float), "-INFINITY sign bit was positive.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x8000, (0x8000 & packed_float), "-INFINITY sign bit was positive.");
 }
 
 static void testNunavutFloat16Pack_zero(void)
 {
     uint16_t packed_float = nunavutFloat16Pack(0.0f);
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x3FF & packed_float), "0.0f had bits in significand.");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x7C00 & packed_float), "0.0f had bits in exponent.");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x80000 & packed_float), "0.0f sign bit was negative.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x03FF & packed_float), "0.0f had bits in significand.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x7C00 & packed_float), "0.0f had bits in exponent.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x8000 & packed_float), "0.0f sign bit was negative.");
 
     packed_float = nunavutFloat16Pack(-0.0f);
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x3FF & packed_float), "-0.0f had bits in significand.");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0, (0x7C00 & packed_float), "-0.0f had bits in exponent.");
-    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x80000, (0x80000 & packed_float), "-0.0f sign bit was not negative.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x03FF & packed_float), "-0.0f had bits in significand.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x0000, (0x7C00 & packed_float), "-0.0f had bits in exponent.");
+    TEST_ASSERT_EQUAL_HEX16_MESSAGE(0x8000, (0x8000 & packed_float), "-0.0f sign bit was not negative.");
 }
 
 // +--------------------------------------------------------------------------+

I'll add this fix to my PR to add C++ support #236

@pavel-kirienko pavel-kirienko added the bug Something isn't working label Feb 25, 2022
@pavel-kirienko
Copy link
Member

That's wild. Can you maybe extract this fix into a separate PR? Should be a no-brainer to do and much easier to accept. Thanks!

asmfreak added a commit to asmfreak/nunavut that referenced this issue Feb 25, 2022
asmfreak added a commit to asmfreak/nunavut that referenced this issue Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build and test trivial
Projects
None yet
Development

No branches or pull requests

2 participants