Skip to content

Conversation

@tbaederr
Copy link
Contributor

@tbaederr tbaederr commented Dec 8, 2024

In certain cases (i.e. long double on x86), the bit with we get from the floating point semantics is different than the type size we compute for the BitCast instruction. Pass this along to DoBitCast, so in there we can check only the relevant bits for being initialized.

This also fixes a weirdness we still had in DoBitCast.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 8, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

In certain cases (i.e. long double on x86), the bit with we get from the floating point semantics is different than the type size we compute for the BitCast instruction. Pass this along to DoBitCast, so in there we can check only the relevant bits for being initialized.

This also fixes a weirdness we still had in DoBitCast.


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

5 Files Affected:

  • (modified) clang/lib/AST/ByteCode/BitcastBuffer.h (+1)
  • (modified) clang/lib/AST/ByteCode/Interp.h (+12-11)
  • (modified) clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp (+12-7)
  • (modified) clang/lib/AST/ByteCode/InterpBuiltinBitCast.h (+3-1)
  • (modified) clang/test/AST/ByteCode/builtin-bit-cast-long-double.cpp (+7)
diff --git a/clang/lib/AST/ByteCode/BitcastBuffer.h b/clang/lib/AST/ByteCode/BitcastBuffer.h
index 2a0d8a0cd9a81f..b1b6b9e5173a7c 100644
--- a/clang/lib/AST/ByteCode/BitcastBuffer.h
+++ b/clang/lib/AST/ByteCode/BitcastBuffer.h
@@ -45,6 +45,7 @@ struct Bits {
   bool operator>=(Bits Other) const { return N >= Other.N; }
   bool operator<=(Bits Other) const { return N <= Other.N; }
   bool operator==(Bits Other) const { return N == Other.N; }
+  bool operator!=(Bits Other) const { return N != Other.N; }
 };
 
 /// A quantity in bytes.
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index c5c2a5ef19cc4d..97e954112ae281 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_AST_INTERP_INTERP_H
 
 #include "../ExprConstShared.h"
+#include "BitcastBuffer.h"
 #include "Boolean.h"
 #include "DynamicAllocator.h"
 #include "FixedPoint.h"
@@ -3050,7 +3051,16 @@ inline bool BitCast(InterpState &S, CodePtr OpPC, bool TargetIsUCharOrByte,
   llvm::SmallVector<std::byte> Buff(BuffSize);
   bool HasIndeterminateBits = false;
 
-  if (!DoBitCast(S, OpPC, FromPtr, Buff.data(), BuffSize, HasIndeterminateBits))
+  Bits FullBitWidth(ResultBitWidth);
+  Bits BitWidth = FullBitWidth;
+
+  if constexpr (std::is_same_v<T, Floating>) {
+    assert(Sem);
+    BitWidth = Bits(llvm::APFloatBase::getSizeInBits(*Sem));
+  }
+
+  if (!DoBitCast(S, OpPC, FromPtr, Buff.data(), FullBitWidth, BitWidth,
+                 HasIndeterminateBits))
     return false;
 
   if (!CheckBitCast(S, OpPC, HasIndeterminateBits, TargetIsUCharOrByte))
@@ -3058,16 +3068,7 @@ inline bool BitCast(InterpState &S, CodePtr OpPC, bool TargetIsUCharOrByte,
 
   if constexpr (std::is_same_v<T, Floating>) {
     assert(Sem);
-    ptrdiff_t Offset = 0;
-
-    if (llvm::sys::IsBigEndianHost) {
-      unsigned NumBits = llvm::APFloatBase::getSizeInBits(*Sem);
-      assert(NumBits % 8 == 0);
-      assert(NumBits <= ResultBitWidth);
-      Offset = (ResultBitWidth - NumBits) / 8;
-    }
-
-    S.Stk.push<Floating>(T::bitcastFromMemory(Buff.data() + Offset, *Sem));
+    S.Stk.push<Floating>(T::bitcastFromMemory(Buff.data(), *Sem));
   } else {
     assert(!Sem);
     S.Stk.push<T>(T::bitcastFromMemory(Buff.data(), ResultBitWidth));
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
index e12babc162ce3c..915d0723bf5363 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
@@ -278,44 +278,49 @@ static bool readPointerToBuffer(const Context &Ctx, const Pointer &FromPtr,
           if (llvm::sys::IsBigEndianHost)
             swapBytes(Buff.get(), NumBits.roundToBytes());
 
+          Buffer.markInitialized(BitOffset, NumBits);
         } else {
           BITCAST_TYPE_SWITCH(T, { P.deref<T>().bitcastToMemory(Buff.get()); });
 
           if (llvm::sys::IsBigEndianHost)
             swapBytes(Buff.get(), FullBitWidth.roundToBytes());
+          Buffer.markInitialized(BitOffset, BitWidth);
         }
 
         Buffer.pushData(Buff.get(), BitOffset, BitWidth, TargetEndianness);
-        Buffer.markInitialized(BitOffset, BitWidth);
         return true;
       });
 }
 
 bool clang::interp::DoBitCast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-                              std::byte *Buff, size_t BuffSize,
+                              std::byte *Buff, Bits FullBitWidth, Bits BitWidth,
                               bool &HasIndeterminateBits) {
   assert(Ptr.isLive());
   assert(Ptr.isBlockPointer());
   assert(Buff);
+  assert(BitWidth <= FullBitWidth);
+  assert(FullBitWidth.isFullByte());
+  assert(BitWidth.isFullByte());
 
-  Bits BitSize = Bytes(BuffSize).toBits();
-  BitcastBuffer Buffer(BitSize);
+  BitcastBuffer Buffer(FullBitWidth);
+  size_t BuffSize = FullBitWidth.roundToBytes();
   if (!CheckBitcastType(S, OpPC, Ptr.getType(), /*IsToType=*/false))
     return false;
 
   bool Success = readPointerToBuffer(S.getContext(), Ptr, Buffer,
                                      /*ReturnOnUninit=*/false);
-  HasIndeterminateBits = !Buffer.allInitialized();
+  HasIndeterminateBits = !Buffer.rangeInitialized(Bits::zero(), BitWidth);
 
   const ASTContext &ASTCtx = S.getASTContext();
   Endian TargetEndianness =
       ASTCtx.getTargetInfo().isLittleEndian() ? Endian::Little : Endian::Big;
-  auto B = Buffer.copyBits(Bits::zero(), BitSize, BitSize, TargetEndianness);
+  auto B =
+      Buffer.copyBits(Bits::zero(), BitWidth, FullBitWidth, TargetEndianness);
 
   std::memcpy(Buff, B.get(), BuffSize);
 
   if (llvm::sys::IsBigEndianHost)
-    swapBytes(Buff, BuffSize);
+    swapBytes(Buff, BitWidth.roundToBytes());
 
   return Success;
 }
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h
index 8142e0bf28fcf2..6f9f0430c5b70d 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_AST_INTERP_BUILITN_BIT_CAST_H
 #define LLVM_CLANG_AST_INTERP_BUILITN_BIT_CAST_H
 
+#include "BitcastBuffer.h"
 #include <cstddef>
 
 namespace clang {
@@ -18,7 +19,8 @@ class InterpState;
 class CodePtr;
 
 bool DoBitCast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-               std::byte *Buff, size_t BuffSize, bool &HasIndeterminateBits);
+               std::byte *Buff, Bits FullBitWidth, Bits BitWidth,
+               bool &HasIndeterminateBits);
 bool DoBitCastPtr(InterpState &S, CodePtr OpPC, const Pointer &FromPtr,
                   Pointer &ToPtr);
 bool DoBitCastPtr(InterpState &S, CodePtr OpPC, const Pointer &FromPtr,
diff --git a/clang/test/AST/ByteCode/builtin-bit-cast-long-double.cpp b/clang/test/AST/ByteCode/builtin-bit-cast-long-double.cpp
index 0929f7cb70b744..fa02ec81f64495 100644
--- a/clang/test/AST/ByteCode/builtin-bit-cast-long-double.cpp
+++ b/clang/test/AST/ByteCode/builtin-bit-cast-long-double.cpp
@@ -49,6 +49,13 @@ static_assert(round_trip<bytes>(ld), "");
 
 static_assert(round_trip<long double>(10.0L));
 
+constexpr long double foo() {
+  bytes A = __builtin_bit_cast(bytes, ld);
+  long double ld = __builtin_bit_cast(long double, A);
+  return ld;
+}
+static_assert(foo() == ld);
+
 #if 0
 constexpr bool f(bool read_uninit) {
   bytes b = bit_cast<bytes>(ld);

In certain cases (i.e. long double on x86), the bit with we get from
the floating point semantics is different than the type size we
compute for the BitCast instruction. Pass this along to DoBitCast,
so in there we can check only the relevant bits for being initialized.

This also fixes a weirdness we still had in DoBitCast.
@tbaederr tbaederr merged commit 1fbbf4c into llvm:main Dec 8, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants