[libc-commits] [llvm] [libc] [reland][libc] Make BigInt bit_cast-able to compatible types (PR #74862)

Guillaume Chatelet via libc-commits libc-commits at lists.llvm.org
Fri Dec 8 08:42:28 PST 2023


https://github.com/gchatelet created https://github.com/llvm/llvm-project/pull/74862

Fix #74258
This is a reland of #74837, the error went unnoticed because it compiles fine on
clang-16 but not on clang-12 which is the version used on the buildbots.

The fix was to explicitly initialize `BigInt` variables in `constexpr` operations: `BigInt<Bits, Signed> result(0);` instead of `BigInt<Bits, Signed> result;`


>From a03035813d8486ad1b51dc18ba0fb68631028ebc Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Fri, 8 Dec 2023 16:36:46 +0000
Subject: [PATCH] [reland][libc] Make BigInt bit_cast-able to compatible types

Fix #74258
This is a reland of #74837
---
 libc/src/__support/UInt.h                     | 21 +++---
 libc/test/src/__support/CMakeLists.txt        |  5 +-
 libc/test/src/__support/uint_test.cpp         | 70 ++++++++++++++++---
 .../libc/test/src/__support/BUILD.bazel       |  3 +
 4 files changed, 79 insertions(+), 20 deletions(-)

diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index f72b995f8788d..bdc713e6a8db2 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -25,12 +25,13 @@
 
 namespace LIBC_NAMESPACE::cpp {
 
+// BigInt has the same semantics as the 'standard integer types' and can be
+// safely 'bit_cast'ed to compatible types.
 template <size_t Bits, bool Signed> struct BigInt {
-
   static_assert(Bits > 0 && Bits % 64 == 0,
                 "Number of bits in BigInt should be a multiple of 64.");
   LIBC_INLINE_VAR static constexpr size_t WORDCOUNT = Bits / 64;
-  uint64_t val[WORDCOUNT]{};
+  uint64_t val[WORDCOUNT];
 
   LIBC_INLINE_VAR static constexpr uint64_t MASK32 = 0xFFFFFFFFu;
 
@@ -158,7 +159,7 @@ template <size_t Bits, bool Signed> struct BigInt {
 
   LIBC_INLINE constexpr BigInt<Bits, Signed>
   operator+(const BigInt<Bits, Signed> &other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     SumCarry<uint64_t> s{0, 0};
     for (size_t i = 0; i < WORDCOUNT; ++i) {
       s = add_with_carry(val[i], other.val[i], s.carry);
@@ -171,7 +172,7 @@ template <size_t Bits, bool Signed> struct BigInt {
   // it will always use the constexpr version of add_with_carry.
   LIBC_INLINE constexpr BigInt<Bits, Signed>
   operator+(BigInt<Bits, Signed> &&other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     SumCarry<uint64_t> s{0, 0};
     for (size_t i = 0; i < WORDCOUNT; ++i) {
       s = add_with_carry_const(val[i], other.val[i], s.carry);
@@ -199,7 +200,7 @@ template <size_t Bits, bool Signed> struct BigInt {
 
   LIBC_INLINE BigInt<Bits, Signed>
   operator-(const BigInt<Bits, Signed> &other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     DiffBorrow<uint64_t> d{0, 0};
     for (size_t i = 0; i < WORDCOUNT; ++i) {
       d = sub_with_borrow(val[i], other.val[i], d.borrow);
@@ -210,7 +211,7 @@ template <size_t Bits, bool Signed> struct BigInt {
 
   LIBC_INLINE constexpr BigInt<Bits, Signed>
   operator-(BigInt<Bits, Signed> &&other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     DiffBorrow<uint64_t> d{0, 0};
     for (size_t i = 0; i < WORDCOUNT; ++i) {
       d = sub_with_borrow_const(val[i], other.val[i], d.borrow);
@@ -690,7 +691,7 @@ template <size_t Bits, bool Signed> struct BigInt {
 
   LIBC_INLINE constexpr BigInt<Bits, Signed>
   operator&(const BigInt<Bits, Signed> &other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     for (size_t i = 0; i < WORDCOUNT; ++i)
       result.val[i] = val[i] & other.val[i];
     return result;
@@ -705,7 +706,7 @@ template <size_t Bits, bool Signed> struct BigInt {
 
   LIBC_INLINE constexpr BigInt<Bits, Signed>
   operator|(const BigInt<Bits, Signed> &other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     for (size_t i = 0; i < WORDCOUNT; ++i)
       result.val[i] = val[i] | other.val[i];
     return result;
@@ -720,7 +721,7 @@ template <size_t Bits, bool Signed> struct BigInt {
 
   LIBC_INLINE constexpr BigInt<Bits, Signed>
   operator^(const BigInt<Bits, Signed> &other) const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     for (size_t i = 0; i < WORDCOUNT; ++i)
       result.val[i] = val[i] ^ other.val[i];
     return result;
@@ -734,7 +735,7 @@ template <size_t Bits, bool Signed> struct BigInt {
   }
 
   LIBC_INLINE constexpr BigInt<Bits, Signed> operator~() const {
-    BigInt<Bits, Signed> result;
+    BigInt<Bits, Signed> result(0);
     for (size_t i = 0; i < WORDCOUNT; ++i)
       result.val[i] = ~val[i];
     return result;
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index 740209bc83d75..187b70781bdad 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -89,8 +89,11 @@ add_libc_test(
   SRCS
     uint_test.cpp
   DEPENDS
-    libc.src.__support.uint
+    libc.src.__support.CPP.bit
     libc.src.__support.CPP.optional
+    libc.src.__support.CPP.type_traits
+    libc.src.__support.macros.properties.float
+    libc.src.__support.uint
 )
 
 add_libc_test(
diff --git a/libc/test/src/__support/uint_test.cpp b/libc/test/src/__support/uint_test.cpp
index 971bac55bd9d3..e4d24b08da867 100644
--- a/libc/test/src/__support/uint_test.cpp
+++ b/libc/test/src/__support/uint_test.cpp
@@ -6,23 +6,73 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/__support/CPP/bit.h" // bit_cast
 #include "src/__support/CPP/optional.h"
+#include "src/__support/CPP/type_traits.h" // is_trivially_constructible
 #include "src/__support/UInt.h"
+#include "src/__support/macros/properties/float.h" // LIBC_COMPILER_HAS_FLOAT128
 
 #include "test/UnitTest/Test.h"
 
-// We want to test LIBC_NAMESPACE::cpp::UInt<128> explicitly. So, for
+#include <math.h> // HUGE_VALF, HUGE_VALF
+
+namespace LIBC_NAMESPACE {
+
+using LL_UInt64 = cpp::UInt<64>;
+// We want to test cpp::UInt<128> explicitly. So, for
 // convenience, we use a sugar which does not conflict with the UInt128 type
 // which can resolve to __uint128_t if the platform has it.
-using LL_UInt128 = LIBC_NAMESPACE::cpp::UInt<128>;
-using LL_UInt192 = LIBC_NAMESPACE::cpp::UInt<192>;
-using LL_UInt256 = LIBC_NAMESPACE::cpp::UInt<256>;
-using LL_UInt320 = LIBC_NAMESPACE::cpp::UInt<320>;
-using LL_UInt512 = LIBC_NAMESPACE::cpp::UInt<512>;
-using LL_UInt1024 = LIBC_NAMESPACE::cpp::UInt<1024>;
+using LL_UInt128 = cpp::UInt<128>;
+using LL_UInt192 = cpp::UInt<192>;
+using LL_UInt256 = cpp::UInt<256>;
+using LL_UInt320 = cpp::UInt<320>;
+using LL_UInt512 = cpp::UInt<512>;
+using LL_UInt1024 = cpp::UInt<1024>;
+
+using LL_Int128 = cpp::Int<128>;
+using LL_Int192 = cpp::Int<192>;
+
+TEST(LlvmLibcUIntClassTest, BitCastToFromDouble) {
+  static_assert(cpp::is_trivially_constructible<LL_UInt64>::value);
+  static_assert(cpp::is_trivially_copyable<LL_UInt64>::value);
+  static_assert(sizeof(LL_UInt64) == sizeof(double));
+  const double inf = HUGE_VAL;
+  const double max = DBL_MAX;
+  const double array[] = {0.0, 0.1, 1.0, max, inf};
+  for (double value : array) {
+    LL_UInt64 back = cpp::bit_cast<LL_UInt64>(value);
+    double forth = cpp::bit_cast<double>(back);
+    EXPECT_TRUE(value == forth);
+  }
+}
 
-using LL_Int128 = LIBC_NAMESPACE::cpp::Int<128>;
-using LL_Int192 = LIBC_NAMESPACE::cpp::Int<192>;
+#ifdef __SIZEOF_INT128__
+TEST(LlvmLibcUIntClassTest, BitCastToFromNativeUint128) {
+  static_assert(cpp::is_trivially_constructible<LL_UInt128>::value);
+  static_assert(cpp::is_trivially_copyable<LL_UInt128>::value);
+  static_assert(sizeof(LL_UInt128) == sizeof(__uint128_t));
+  const __uint128_t array[] = {0, 1, ~__uint128_t(0)};
+  for (__uint128_t value : array) {
+    LL_UInt128 back = cpp::bit_cast<LL_UInt128>(value);
+    __uint128_t forth = cpp::bit_cast<__uint128_t>(back);
+    EXPECT_TRUE(value == forth);
+  }
+}
+#endif
+
+#ifdef LIBC_COMPILER_HAS_FLOAT128
+TEST(LlvmLibcUIntClassTest, BitCastToFromNativeFloat128) {
+  static_assert(cpp::is_trivially_constructible<LL_UInt128>::value);
+  static_assert(cpp::is_trivially_copyable<LL_UInt128>::value);
+  static_assert(sizeof(LL_UInt128) == sizeof(float128));
+  const float128 array[] = {0, 0.1, 1};
+  for (float128 value : array) {
+    LL_UInt128 back = cpp::bit_cast<LL_UInt128>(value);
+    float128 forth = cpp::bit_cast<float128>(back);
+    EXPECT_TRUE(value == forth);
+  }
+}
+#endif
 
 TEST(LlvmLibcUIntClassTest, BasicInit) {
   LL_UInt128 half_val(12345);
@@ -634,3 +684,5 @@ TEST(LlvmLibcUIntClassTest, ConstructorFromUInt128Tests) {
 }
 
 #endif // __SIZEOF_INT128__
+
+} // namespace LIBC_NAMESPACE
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
index a973e6541da01..f775183296636 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
@@ -74,7 +74,10 @@ libc_test(
     name = "uint_test",
     srcs = ["uint_test.cpp"],
     deps = [
+        "//libc:__support_cpp_bit",
         "//libc:__support_cpp_optional",
+        "//libc:__support_cpp_type_traits",
+        "//libc:__support_macros_properties_float",
         "//libc:__support_uint",
     ],
 )



More information about the libc-commits mailing list