[libc-commits] [libc] [libc] Fix a bug in fx_bits.h due to integer promotion of bitwise ops. (PR #83647)

via libc-commits libc-commits at lists.llvm.org
Fri Mar 1 18:45:35 PST 2024


https://github.com/lntue created https://github.com/llvm/llvm-project/pull/83647

None

>From 27cf396ba401615db7fcc75f6fe42c4939d5d7f2 Mon Sep 17 00:00:00 2001
From: Tue Ly <lntue.h at gmail.com>
Date: Sat, 2 Mar 2024 02:42:17 +0000
Subject: [PATCH] [libc] Fix a bug in fx_bits due to integer promotion of
 bitwise ops.

---
 libc/src/__support/fixed_point/fx_bits.h      |  2 +-
 .../__support/fixed_point/fx_bits_test.cpp    | 77 +++++++++++++++----
 2 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/libc/src/__support/fixed_point/fx_bits.h b/libc/src/__support/fixed_point/fx_bits.h
index 41da45c01e4e19..53e693d4ddfd12 100644
--- a/libc/src/__support/fixed_point/fx_bits.h
+++ b/libc/src/__support/fixed_point/fx_bits.h
@@ -126,7 +126,7 @@ bit_not(T x) {
   using BitType = typename FXRep<T>::StorageType;
   BitType x_bit = cpp::bit_cast<BitType>(x);
   // For some reason, bit_cast cannot deduce BitType from the input.
-  return cpp::bit_cast<T, BitType>(~x_bit);
+  return cpp::bit_cast<T, BitType>(static_cast<BitType>(~x_bit));
 }
 
 template <typename T> LIBC_INLINE constexpr T abs(T x) {
diff --git a/libc/test/src/__support/fixed_point/fx_bits_test.cpp b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
index 58627816eb8d97..3cbd800adc3c35 100644
--- a/libc/test/src/__support/fixed_point/fx_bits_test.cpp
+++ b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
@@ -20,9 +20,22 @@ using LIBC_NAMESPACE::operator""_u16;
 using LIBC_NAMESPACE::operator""_u32;
 using LIBC_NAMESPACE::operator""_u64;
 
+class LlvmLibcFxBitsTest : public LIBC_NAMESPACE::testing::Test {
+public:
+  template <typename T> void testBitwiseOps() {
+    EXPECT_EQ(LIBC_NAMESPACE::fixed_point::bit_and(T(0.75), T(0.375)), T(0.25));
+    EXPECT_EQ(LIBC_NAMESPACE::fixed_point::bit_or(T(0.75), T(0.375)), T(0.875));
+    using StorageType = typename FXRep<T>::StorageType;
+    StorageType a = LIBC_NAMESPACE::cpp::bit_cast<StorageType>(T(0.75));
+    a = ~a;
+    EXPECT_EQ(LIBC_NAMESPACE::fixed_point::bit_not(T(0.75)),
+              FXBits<T>(a).get_val());
+  }
+};
+
 // -------------------------------- SHORT TESTS --------------------------------
 
-TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
   auto bits_var = FXBits<unsigned short fract>(0b00000000_u8);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -51,9 +64,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
   EXPECT_EQ(bits_var.get_sign(), false);
   EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
   EXPECT_EQ(bits_var.get_fraction(), 0xcd_u8);
+
+  // Bitwise ops
+  testBitwiseOps<unsigned short fract>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortAccum) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_UnsignedShortAccum) {
   auto bits_var = FXBits<unsigned short accum>(0b00000000'00000000_u16);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -77,9 +93,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortAccum) {
   EXPECT_EQ(bits_var.get_sign(), false);
   EXPECT_EQ(bits_var.get_integral(), 0x00cd_u16);
   EXPECT_EQ(bits_var.get_fraction(), 0x00fe_u16);
+
+  // Bitwise ops
+  testBitwiseOps<unsigned short accum>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_ShortFract) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_ShortFract) {
   auto bits_var = FXBits<short fract>(0b0'0000000_u8);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -103,9 +122,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_ShortFract) {
   EXPECT_EQ(bits_var.get_sign(), true);
   EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
   EXPECT_EQ(bits_var.get_fraction(), 0x4d_u8);
+
+  // Bitwise ops
+  testBitwiseOps<short fract>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_ShortAccum) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_ShortAccum) {
   auto bits_var = FXBits<short accum>(0b0'00000000'0000000_u16);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -129,9 +151,14 @@ TEST(LlvmLibcFxBitsTest, FXBits_ShortAccum) {
   EXPECT_EQ(bits_var.get_sign(), true);
   EXPECT_EQ(bits_var.get_integral(), 0x00cd_u16);
   EXPECT_EQ(bits_var.get_fraction(), 0x007e_u16);
+
+  // Bitwise ops
+  testBitwiseOps<short accum>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_UnsignedFract) {
+// -------------------------------- NORMAL TESTS -------------------------------
+
+TEST_F(LlvmLibcFxBitsTest, FXBits_UnsignedFract) {
   auto bits_var = FXBits<unsigned fract>(0b0000000000000000_u16);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -155,11 +182,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedFract) {
   EXPECT_EQ(bits_var.get_sign(), false);
   EXPECT_EQ(bits_var.get_integral(), 0x0000_u16);
   EXPECT_EQ(bits_var.get_fraction(), 0xef12_u16);
-}
 
-// -------------------------------- NORMAL TESTS -------------------------------
+  // Bitwise ops
+  testBitwiseOps<unsigned fract>();
+}
 
-TEST(LlvmLibcFxBitsTest, FXBits_UnsignedAccum) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_UnsignedAccum) {
   auto bits_var =
       FXBits<unsigned accum>(0b0000000000000000'0000000000000000_u32);
 
@@ -184,9 +212,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedAccum) {
   EXPECT_EQ(bits_var.get_sign(), false);
   EXPECT_EQ(bits_var.get_integral(), 0x0000abcd_u32);
   EXPECT_EQ(bits_var.get_fraction(), 0x0000ef12_u32);
+
+  // Bitwise ops
+  testBitwiseOps<unsigned accum>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_Fract) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_Fract) {
   auto bits_var = FXBits<fract>(0b0'000000000000000_u16);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -210,9 +241,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_Fract) {
   EXPECT_EQ(bits_var.get_sign(), true);
   EXPECT_EQ(bits_var.get_integral(), 0x0000_u16);
   EXPECT_EQ(bits_var.get_fraction(), 0x6f12_u16);
+
+  // Bitwise ops
+  testBitwiseOps<fract>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_Accum) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_Accum) {
   auto bits_var = FXBits<accum>(0b0'0000000000000000'000000000000000_u32);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -236,11 +270,14 @@ TEST(LlvmLibcFxBitsTest, FXBits_Accum) {
   EXPECT_EQ(bits_var.get_sign(), true);
   EXPECT_EQ(bits_var.get_integral(), 0x0000abcd_u32);
   EXPECT_EQ(bits_var.get_fraction(), 0x00006f12_u32);
+
+  // Bitwise ops
+  testBitwiseOps<accum>();
 }
 
 // --------------------------------- LONG TESTS --------------------------------
 
-TEST(LlvmLibcFxBitsTest, FXBits_UnsignedLongFract) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_UnsignedLongFract) {
   auto bits_var =
       FXBits<unsigned long fract>(0b00000000000000000000000000000000_u32);
 
@@ -265,9 +302,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedLongFract) {
   EXPECT_EQ(bits_var.get_sign(), false);
   EXPECT_EQ(bits_var.get_integral(), 0x00000000_u32);
   EXPECT_EQ(bits_var.get_fraction(), 0xfedcba98_u32);
+
+  // Bitwise ops
+  testBitwiseOps<unsigned long fract>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_UnsignedLongAccum) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_UnsignedLongAccum) {
   auto bits_var = FXBits<unsigned long accum>(
       0b00000000000000000000000000000000'00000000000000000000000000000000_u64);
 
@@ -292,9 +332,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedLongAccum) {
   EXPECT_EQ(bits_var.get_sign(), false);
   EXPECT_EQ(bits_var.get_integral(), 0x00000000abcdef12_u64);
   EXPECT_EQ(bits_var.get_fraction(), 0x00000000fedcba98_u64);
+
+  // Bitwise ops
+  testBitwiseOps<unsigned long accum>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_LongFract) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_LongFract) {
   auto bits_var = FXBits<long fract>(0b0'0000000000000000000000000000000_u32);
 
   EXPECT_EQ(bits_var.get_sign(), false);
@@ -318,9 +361,12 @@ TEST(LlvmLibcFxBitsTest, FXBits_LongFract) {
   EXPECT_EQ(bits_var.get_sign(), true);
   EXPECT_EQ(bits_var.get_integral(), 0x00000000_u32);
   EXPECT_EQ(bits_var.get_fraction(), 0x7edcba98_u32);
+
+  // Bitwise ops
+  testBitwiseOps<long fract>();
 }
 
-TEST(LlvmLibcFxBitsTest, FXBits_LongAccum) {
+TEST_F(LlvmLibcFxBitsTest, FXBits_LongAccum) {
   auto bits_var = FXBits<long accum>(
       0b0'00000000000000000000000000000000'0000000000000000000000000000000_u64);
 
@@ -345,4 +391,7 @@ TEST(LlvmLibcFxBitsTest, FXBits_LongAccum) {
   EXPECT_EQ(bits_var.get_sign(), true);
   EXPECT_EQ(bits_var.get_integral(), 0x00000000abcdef12_u64);
   EXPECT_EQ(bits_var.get_fraction(), 0x000000007edcba98_u64);
+
+  // Bitwise ops
+  testBitwiseOps<long accum>();
 }



More information about the libc-commits mailing list