Skip to content

[libc][NFC] fix uint128 init in float properties #75084

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

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

michaelrj-google
Copy link
Contributor

An unsigned integer was being initialized with -1 to set all its bits to
1, but this doesn't work for the BigInt class which may be used as an
integer type on systems that don't support uint128 natively. This patch
moves the initialization to use ~T(0) instead.

An unsigned integer was being initialized with -1 to set all its bits to
1, but this doesn't work for the BigInt class which may be used as an
integer type on systems that don't support uint128 natively. This patch
moves the initialization to use ~T(0) instead.
@llvmbot llvmbot added the libc label Dec 11, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2023

@llvm/pr-subscribers-libc

Author: None (michaelrj-google)

Changes

An unsigned integer was being initialized with -1 to set all its bits to
1, but this doesn't work for the BigInt class which may be used as an
integer type on systems that don't support uint128 natively. This patch
moves the initialization to use ~T(0) instead.


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

1 Files Affected:

  • (modified) libc/src/__support/FPUtil/FloatProperties.h (+4-2)
diff --git a/libc/src/__support/FPUtil/FloatProperties.h b/libc/src/__support/FPUtil/FloatProperties.h
index ba54e9d1781cab..ef6a3cd403db61 100644
--- a/libc/src/__support/FPUtil/FloatProperties.h
+++ b/libc/src/__support/FPUtil/FloatProperties.h
@@ -85,7 +85,9 @@ template <typename T, size_t count> static constexpr T mask_trailing_ones() {
   static_assert(cpp::is_unsigned_v<T>);
   constexpr unsigned t_bits = CHAR_BIT * sizeof(T);
   static_assert(count <= t_bits && "Invalid bit index");
-  return count == 0 ? 0 : (T(-1) >> (t_bits - count));
+  // It's important not to initialize T with -1, since T may be BigInt which
+  // will take -1 as a uint64_t and only initialize the low 64 bits.
+  return count == 0 ? 0 : ((~T(0)) >> (t_bits - count));
 }
 
 // Derives more properties from 'FPBaseProperties' above.
@@ -131,7 +133,7 @@ struct FPCommonProperties : private FPBaseProperties<fp_type> {
   LIBC_INLINE_VAR static constexpr UIntType FP_MASK =
       mask_trailing_ones<UIntType, TOTAL_BITS>();
   static_assert((SIG_MASK & EXP_MASK & SIGN_MASK_) == 0, "masks disjoint");
-  static_assert((SIG_MASK | EXP_MASK | SIGN_MASK_) == FP_MASK, "masks covers");
+  static_assert((SIG_MASK | EXP_MASK | SIGN_MASK_) == FP_MASK, "masks cover");
 
   LIBC_INLINE static constexpr UIntType bit_at(int position) {
     return UIntType(1) << position;

@michaelrj-google michaelrj-google merged commit b1a91b7 into llvm:main Dec 11, 2023
@michaelrj-google michaelrj-google deleted the libcFixFloatProperties branch December 11, 2023 18:46
@gchatelet
Copy link
Contributor

Appeared in #75063 (comment).

Thx for the quick fix @michaelrj-google !

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.

3 participants