[libc-commits] [libc] [libc] Make BigInt bit_cast-able to compatible types (PR #75063)
Guillaume Chatelet via libc-commits
libc-commits at lists.llvm.org
Tue Dec 19 04:41:22 PST 2023
https://github.com/gchatelet updated https://github.com/llvm/llvm-project/pull/75063
>From 8120cab1c0375b6689341975566e74dee895b91c Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Mon, 11 Dec 2023 15:51:35 +0000
Subject: [PATCH 1/3] [libc] Make BigInt bit_cast-able to compatible types
This is a second take on #74837 to fix #74258
---
libc/src/__support/CPP/bit.h | 30 +++++++++----
libc/src/__support/FPUtil/FPBits.h | 1 +
libc/src/__support/UInt.h | 29 ++++++++++++
libc/src/string/memory_utils/utils.h | 27 +-----------
libc/test/src/__support/uint_test.cpp | 63 +++++++++++++++++++++++----
5 files changed, 108 insertions(+), 42 deletions(-)
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index 2091c3662d5076..f9bb2ef9f8332b 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -25,6 +25,27 @@ namespace LIBC_NAMESPACE::cpp {
#define LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
#endif
+// Performs a copy from 'src' to 'dst' of 'size' bytes.
+// The semantics is valid if 'src' and 'dst' are equal but undefined if the
+// regions defined by [src, src + size] and [dst, dst + size] overlap.
+template <size_t size, typename DstT, typename SrcT>
+LIBC_INLINE constexpr void memcpy_inline(DstT *__restrict dst,
+ const SrcT *__restrict src) {
+#ifdef LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
+ __builtin_memcpy(dst, src, size);
+#else
+ // `memcpy_inline` is instantiated several times with different value of the
+ // size parameter. This doesn't play well with GCC's Value Range Analysis that
+ // wrongly detects out of bounds accesses. We disable the 'array-bounds'
+ // warning for the purpose of this function.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+ for (size_t i = 0; i < size; ++i)
+ static_cast<char *>(dst)[i] = static_cast<const char *>(src)[i];
+#pragma GCC diagnostic pop
+#endif
+}
+
// This implementation of bit_cast requires trivially-constructible To, to avoid
// UB in the implementation.
template <
@@ -39,14 +60,7 @@ LIBC_INLINE constexpr To bit_cast(const From &from) {
return __builtin_bit_cast(To, from);
#else
To to;
- char *dst = reinterpret_cast<char *>(&to);
- const char *src = reinterpret_cast<const char *>(&from);
-#if LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
- __builtin_memcpy_inline(dst, src, sizeof(To));
-#else
- for (unsigned i = 0; i < sizeof(To); ++i)
- dst[i] = src[i];
-#endif // LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
+ memcpy_inline<sizeof(To)>(&to, &from);
return to;
#endif // LIBC_HAS_BUILTIN(__builtin_bit_cast)
}
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index ca98aa71262491..eb75abc379a03d 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -11,6 +11,7 @@
#include "src/__support/CPP/bit.h"
#include "src/__support/CPP/type_traits.h"
+#include "src/__support/UInt128.h"
#include "src/__support/common.h"
#include "src/__support/macros/attributes.h" // LIBC_INLINE
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index f72b995f8788db..78d8213cabcf9a 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -952,6 +952,35 @@ struct make_signed<UInt<Bits>> : type_identity<Int<Bits>> {
"Number of bits in Int should be a multiple of 64.");
};
+namespace internal {
+template <typename T> struct is_custom_uint : cpp::false_type {};
+template <size_t Bits> struct is_custom_uint<UInt<Bits>> : cpp::true_type {};
+} // namespace internal
+
+// bit_cast to UInt
+template <typename To, typename From,
+ typename = cpp::enable_if_t<internal::is_custom_uint<To>::value>,
+ typename = cpp::enable_if_t<sizeof(To) == sizeof(From)>,
+ typename = cpp::enable_if_t<cpp::is_trivially_copyable<From>::value>>
+LIBC_INLINE constexpr To bit_cast(const From &from) {
+ To out;
+ cpp::memcpy_inline<sizeof(out)>(out.val, &from);
+ return out;
+}
+
+// bit_cast from UInt
+template <
+ typename To, size_t Bits,
+ typename = cpp::enable_if_t<sizeof(To) == sizeof(UInt<Bits>)>,
+ typename = cpp::enable_if_t<cpp::is_trivially_constructible<To>::value>,
+ typename = cpp::enable_if_t<cpp::is_trivially_copyable<To>::value>,
+ typename = cpp::enable_if_t<cpp::is_trivially_copyable<UInt<Bits>>::value>>
+LIBC_INLINE constexpr To bit_cast(const UInt<Bits> &from) {
+ To out;
+ cpp::memcpy_inline<sizeof(out)>(&out, from.val);
+ return out;
+}
+
} // namespace LIBC_NAMESPACE::cpp
#endif // LLVM_LIBC_SRC___SUPPORT_UINT_H
diff --git a/libc/src/string/memory_utils/utils.h b/libc/src/string/memory_utils/utils.h
index 5cd716e033d6a4..bbc9f68edb4bd5 100644
--- a/libc/src/string/memory_utils/utils.h
+++ b/libc/src/string/memory_utils/utils.h
@@ -71,33 +71,10 @@ LIBC_INLINE bool is_disjoint(const void *p1, const void *p2, size_t size) {
return sdiff >= 0 ? size <= udiff : size <= neg_udiff;
}
-#if LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
-#define LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
-#endif
-
#if LIBC_HAS_BUILTIN(__builtin_memset_inline)
#define LLVM_LIBC_HAS_BUILTIN_MEMSET_INLINE
#endif
-// Performs a constant count copy.
-template <size_t Size>
-LIBC_INLINE void memcpy_inline(void *__restrict dst,
- const void *__restrict src) {
-#ifdef LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
- __builtin_memcpy_inline(dst, src, Size);
-#else
- // In memory functions `memcpy_inline` is instantiated several times with
- // different value of the Size parameter. This doesn't play well with GCC's
- // Value Range Analysis that wrongly detects out of bounds accesses. We
- // disable the 'array-bounds' warning for the purpose of this function.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Warray-bounds"
- for (size_t i = 0; i < Size; ++i)
- static_cast<char *>(dst)[i] = static_cast<const char *>(src)[i];
-#pragma GCC diagnostic pop
-#endif
-}
-
using Ptr = cpp::byte *; // Pointer to raw data.
using CPtr = const cpp::byte *; // Const pointer to raw data.
@@ -204,13 +181,13 @@ LIBC_INLINE MemcmpReturnType cmp_neq_uint64_t(uint64_t a, uint64_t b) {
// type.
template <typename T> LIBC_INLINE T load(CPtr ptr) {
T Out;
- memcpy_inline<sizeof(T)>(&Out, ptr);
+ cpp::memcpy_inline<sizeof(T)>(&Out, ptr);
return Out;
}
// Stores a value of type T in memory (possibly unaligned).
template <typename T> LIBC_INLINE void store(Ptr ptr, T value) {
- memcpy_inline<sizeof(T)>(ptr, &value);
+ cpp::memcpy_inline<sizeof(T)>(ptr, &value);
}
// On architectures that do not allow for unaligned access we perform several
diff --git a/libc/test/src/__support/uint_test.cpp b/libc/test/src/__support/uint_test.cpp
index 971bac55bd9d3f..0ad72c35645c4b 100644
--- a/libc/test/src/__support/uint_test.cpp
+++ b/libc/test/src/__support/uint_test.cpp
@@ -10,19 +10,62 @@
#include "src/__support/UInt.h"
#include "test/UnitTest/Test.h"
+#include <math.h> // HUGE_VALF, HUGE_VALF
-// We want to test LIBC_NAMESPACE::cpp::UInt<128> explicitly. So, for
+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_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_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_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 +677,5 @@ TEST(LlvmLibcUIntClassTest, ConstructorFromUInt128Tests) {
}
#endif // __SIZEOF_INT128__
+
+} // namespace LIBC_NAMESPACE
>From c6115c2bed919562fd49905e7ce77209615d4d1c Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Fri, 15 Dec 2023 12:41:31 +0000
Subject: [PATCH 2/3] Fix documentation
---
libc/src/__support/CPP/bit.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index f9bb2ef9f8332b..a034b1761ce6ef 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -26,7 +26,7 @@ namespace LIBC_NAMESPACE::cpp {
#endif
// Performs a copy from 'src' to 'dst' of 'size' bytes.
-// The semantics is valid if 'src' and 'dst' are equal but undefined if the
+// The semantics are valid if 'src' and 'dst' are equal but undefined if the
// regions defined by [src, src + size] and [dst, dst + size] overlap.
template <size_t size, typename DstT, typename SrcT>
LIBC_INLINE constexpr void memcpy_inline(DstT *__restrict dst,
>From 583f3b30e61888b5722411e855577d68da2c52e0 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Tue, 19 Dec 2023 12:41:07 +0000
Subject: [PATCH 3/3] Add documentation as to why we need a supplementary
typename
---
libc/src/__support/CPP/bit.h | 8 ++++----
libc/src/__support/UInt.h | 28 +++++++++++++++++++++-------
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index a034b1761ce6ef..490a18eadf6480 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -50,10 +50,10 @@ LIBC_INLINE constexpr void memcpy_inline(DstT *__restrict dst,
// UB in the implementation.
template <
typename To, typename From,
- typename = cpp::enable_if_t<sizeof(To) == sizeof(From)>,
- typename = cpp::enable_if_t<cpp::is_trivially_constructible<To>::value>,
- typename = cpp::enable_if_t<cpp::is_trivially_copyable<To>::value>,
- typename = cpp::enable_if_t<cpp::is_trivially_copyable<From>::value>>
+ typename = cpp::enable_if_t<sizeof(To) == sizeof(From) &&
+ cpp::is_trivially_constructible<To>::value &&
+ cpp::is_trivially_copyable<To>::value &&
+ cpp::is_trivially_copyable<From>::value>>
LIBC_INLINE constexpr To bit_cast(const From &from) {
MSAN_UNPOISON(&from, sizeof(From));
#if LIBC_HAS_BUILTIN(__builtin_bit_cast)
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index 78d8213cabcf9a..3ad04bfd0f8d47 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -958,10 +958,24 @@ template <size_t Bits> struct is_custom_uint<UInt<Bits>> : cpp::true_type {};
} // namespace internal
// bit_cast to UInt
+// Note: The standard scheme for SFINAE selection is to have exactly one
+// function instanciation valid at a time. This is usually done by having a
+// predicate in one function and the negated predicate in the other one.
+// e.g.
+// template<typename = cpp::enable_if_t< is_custom_uint<To>::value == true> ...
+// template<typename = cpp::enable_if_t< is_custom_uint<To>::value == false> ...
+//
+// Unfortunately this would make the default 'cpp::bit_cast' aware of
+// 'is_custom_uint' (or any other customization). To prevent exposing all
+// customizations in the original function, we create a different function with
+// four 'typename's instead of three - otherwise it would be considered as a
+// redeclaration of the same function leading to "error: template parameter
+// redefines default argument".
template <typename To, typename From,
- typename = cpp::enable_if_t<internal::is_custom_uint<To>::value>,
- typename = cpp::enable_if_t<sizeof(To) == sizeof(From)>,
- typename = cpp::enable_if_t<cpp::is_trivially_copyable<From>::value>>
+ typename = cpp::enable_if_t<sizeof(To) == sizeof(From) &&
+ cpp::is_trivially_copyable<To>::value &&
+ cpp::is_trivially_copyable<From>::value>,
+ typename = cpp::enable_if_t<internal::is_custom_uint<To>::value>>
LIBC_INLINE constexpr To bit_cast(const From &from) {
To out;
cpp::memcpy_inline<sizeof(out)>(out.val, &from);
@@ -971,10 +985,10 @@ LIBC_INLINE constexpr To bit_cast(const From &from) {
// bit_cast from UInt
template <
typename To, size_t Bits,
- typename = cpp::enable_if_t<sizeof(To) == sizeof(UInt<Bits>)>,
- typename = cpp::enable_if_t<cpp::is_trivially_constructible<To>::value>,
- typename = cpp::enable_if_t<cpp::is_trivially_copyable<To>::value>,
- typename = cpp::enable_if_t<cpp::is_trivially_copyable<UInt<Bits>>::value>>
+ typename = cpp::enable_if_t<sizeof(To) == sizeof(UInt<Bits>) &&
+ cpp::is_trivially_constructible<To>::value &&
+ cpp::is_trivially_copyable<To>::value &&
+ cpp::is_trivially_copyable<UInt<Bits>>::value>>
LIBC_INLINE constexpr To bit_cast(const UInt<Bits> &from) {
To out;
cpp::memcpy_inline<sizeof(out)>(&out, from.val);
More information about the libc-commits
mailing list