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

[reland][libc] Make BigInt bit_cast-able to compatible types #74862

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

gchatelet
Copy link
Contributor

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;

@llvmbot llvmbot added the libc label Dec 8, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 8, 2023

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

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&lt;Bits, Signed&gt; result(0); instead of BigInt&lt;Bits, Signed&gt; result;


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

4 Files Affected:

  • (modified) libc/src/__support/UInt.h (+11-10)
  • (modified) libc/test/src/__support/CMakeLists.txt (+4-1)
  • (modified) libc/test/src/__support/uint_test.cpp (+61-9)
  • (modified) utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel (+3)
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index f72b995f8788db..bdc713e6a8db2b 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 740209bc83d75e..187b70781bdad7 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 971bac55bd9d3f..e4d24b08da867c 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 a973e6541da015..f775183296636e 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",
     ],
 )

@gchatelet gchatelet merged commit 31316b3 into llvm:main Dec 8, 2023
4 checks passed
gchatelet added a commit that referenced this pull request Dec 8, 2023
@gchatelet
Copy link
Contributor Author

The libc-x86_64-debian-gcc-fullbuild-dbg build bot is still broken.
I'll fix it ASAP.

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.

libc/src/__support/FPUtil/FPBits.h:111:14: error: no matching function for call to bit_cast
2 participants