[clang][bytecode] Handle bitcasts involving bitfields #116843
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesLet's see what the CI has to say about this. Patch is 35.86 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116843.diff 9 Files Affected:
diff --git a/clang/lib/AST/ByteCode/BitcastBuffer.h b/clang/lib/AST/ByteCode/BitcastBuffer.h
new file mode 100644
index 00000000000000..1b2dcfdcd32a41
--- /dev/null
+++ b/clang/lib/AST/ByteCode/BitcastBuffer.h
@@ -0,0 +1,125 @@
+
+
+#ifndef LLVM_CLANG_AST_INTERP_BITCAST_BUFFER_H
+#define LLVM_CLANG_AST_INTERP_BITCAST_BUFFER_H
+
+#include "llvm/Support/raw_ostream.h"
+#include <bitset>
+#include <cassert>
+#include <cstring>
+#include <memory>
+#include <sstream>
+
+enum class Endian { Little, Big };
+
+static inline bool bitof(std::byte B, unsigned BitIndex) {
+ assert(BitIndex < 8);
+ return (B & (std::byte{1} << BitIndex)) != std::byte{0};
+}
+
+static inline bool fullByte(unsigned N) { return N % 8 == 0; }
+
+/// Track what bits have been initialized to known values and which ones
+/// have indeterminate value.
+/// All offsets are in bits.
+struct BitcastBuffer {
+ size_t FinalBitSize = 0;
+ std::unique_ptr<std::byte[]> Data;
+
+ BitcastBuffer(size_t FinalBitSize) : FinalBitSize(FinalBitSize) {
+ assert(fullByte(FinalBitSize));
+ unsigned ByteSize = FinalBitSize / 8;
+ Data = std::make_unique<std::byte[]>(ByteSize);
+ }
+
+ size_t size() const { return FinalBitSize; }
+
+ bool allInitialized() const {
+ // FIXME: Implement.
+ return true;
+ }
+
+ /// \p Data must be in the given endianness.
+ void pushData(const std::byte *data, size_t BitOffset, size_t BitWidth,
+ size_t FullSize, Endian DataEndianness) {
+ assert(fullByte(FullSize));
+
+ for (unsigned It = 0; It != BitWidth; ++It) {
+ bool BitValue;
+ BitValue = bitof(data[It / 8], It % 8);
+ if (!BitValue)
+ continue;
+
+ unsigned DstBit;
+ if (DataEndianness == Endian::Big)
+ DstBit = size() - BitOffset - BitWidth + It;
+ else
+ DstBit = BitOffset + It;
+ unsigned DstByte = (DstBit / 8);
+
+ Data[DstByte] |= std::byte{1} << (DstBit % 8);
+ }
+ }
+
+ std::unique_ptr<std::byte[]> copyBits(unsigned BitOffset, unsigned BitWidth,
+ unsigned FullBitWidth,
+ Endian DataEndianness) const {
+ assert(BitWidth <= FullBitWidth);
+ assert(fullByte(FullBitWidth));
+ std::unique_ptr<std::byte[]> Out =
+ std::make_unique<std::byte[]>(FullBitWidth / 8);
+
+ for (unsigned It = 0; It != BitWidth; ++It) {
+ unsigned BitIndex;
+ if (DataEndianness == Endian::Little)
+ BitIndex = BitOffset + It;
+ else
+ BitIndex = size() - BitWidth - BitOffset + It;
+
+ bool BitValue = bitof(Data[BitIndex / 8], BitIndex % 8);
+ if (!BitValue)
+ continue;
+ unsigned DstBit = It;
+ unsigned DstByte(DstBit / 8);
+ Out[DstByte] |= std::byte{1} << (DstBit % 8);
+ }
+
+ return Out;
+ }
+
+#if 0
+ template<typename T>
+ static std::string hex(T t) {
+ std::stringstream stream;
+ stream << std::hex << (int)t;
+ return std::string(stream.str());
+ }
+
+
+ void dump(bool AsHex = true) const {
+ llvm::errs() << "LSB\n ";
+ unsigned LineLength = 0;
+ for (unsigned I = 0; I != (FinalBitSize / 8); ++I) {
+ std::byte B = Data[I];
+ if (AsHex) {
+ std::stringstream stream;
+ stream << std::hex << (int)B;
+ llvm::errs() << stream.str();
+ LineLength += stream.str().size() + 1;
+ } else {
+ llvm::errs() << std::bitset<8>((int)B).to_string();
+ LineLength += 8 + 1;
+ // llvm::errs() << (int)B;
+ }
+ llvm::errs() << ' ';
+ }
+ llvm::errs() << '\n';
+
+ for (unsigned I = 0; I != LineLength; ++I)
+ llvm::errs() << ' ';
+ llvm::errs() << "MSB\n";
+ }
+#endif
+};
+
+#endif
diff --git a/clang/lib/AST/ByteCode/Boolean.h b/clang/lib/AST/ByteCode/Boolean.h
index 78d75e75c7531a..bd46523f330609 100644
--- a/clang/lib/AST/ByteCode/Boolean.h
+++ b/clang/lib/AST/ByteCode/Boolean.h
@@ -84,7 +84,7 @@ class Boolean final {
static Boolean bitcastFromMemory(const std::byte *Buff, unsigned BitWidth) {
// Boolean width is currently always 8 for all supported targets. If this
// changes we need to get the bool width from the target info.
- assert(BitWidth == 8);
+ // assert(BitWidth == 8);
bool Val = static_cast<bool>(*Buff);
return Boolean(Val);
}
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 7cf2519d6a71fb..e0423dfe2091af 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2710,6 +2710,8 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
return false;
if (!this->visitInitializer(SubExpr))
return false;
+ if (!this->emitFinishInit(SubExpr))
+ return false;
if (IsStatic)
return this->emitInitGlobalTempComp(TempDecl, E);
return true;
diff --git a/clang/lib/AST/ByteCode/Integral.h b/clang/lib/AST/ByteCode/Integral.h
index ca3674263aef4f..bb1688a8a7622c 100644
--- a/clang/lib/AST/ByteCode/Integral.h
+++ b/clang/lib/AST/ByteCode/Integral.h
@@ -181,6 +181,7 @@ template <unsigned Bits, bool Signed> class Integral final {
}
Integral truncate(unsigned TruncBits) const {
+ assert(TruncBits >= 1);
if (TruncBits >= Bits)
return *this;
const ReprT BitMask = (ReprT(1) << ReprT(TruncBits)) - 1;
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
index 7e8853d3469317..b089a9b7469cf4 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
#include "InterpBuiltinBitCast.h"
+#include "BitcastBuffer.h"
#include "Boolean.h"
#include "Context.h"
#include "Floating.h"
@@ -17,6 +18,8 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/RecordLayout.h"
#include "clang/Basic/TargetInfo.h"
+#include <bitset>
+#include <cmath>
using namespace clang;
using namespace clang::interp;
@@ -61,80 +64,11 @@ using DataFunc = llvm::function_ref<bool(const Pointer &P, PrimType Ty,
} \
} while (0)
-static bool bitof(std::byte B, unsigned BitIndex) {
- return (B & (std::byte{1} << BitIndex)) != std::byte{0};
-}
-
static void swapBytes(std::byte *M, size_t N) {
for (size_t I = 0; I != (N / 2); ++I)
std::swap(M[I], M[N - 1 - I]);
}
-/// Track what bits have been initialized to known values and which ones
-/// have indeterminate value.
-/// All offsets are in bits.
-struct BitcastBuffer {
- size_t SizeInBits = 0;
- llvm::SmallVector<std::byte> Data;
-
- BitcastBuffer() = default;
-
- size_t size() const { return SizeInBits; }
-
- const std::byte *data() const { return Data.data(); }
-
- std::byte *getBytes(unsigned BitOffset) const {
- assert(BitOffset % 8 == 0);
- assert(BitOffset < SizeInBits);
- return const_cast<std::byte *>(data() + (BitOffset / 8));
- }
-
- bool allInitialized() const {
- // FIXME: Implement.
- return true;
- }
-
- bool atByteBoundary() const { return (Data.size() * 8) == SizeInBits; }
-
- void pushBit(bool Value) {
- if (atByteBoundary())
- Data.push_back(std::byte{0});
-
- if (Value)
- Data.back() |= (std::byte{1} << (SizeInBits % 8));
- ++SizeInBits;
- }
-
- void pushData(const std::byte *data, size_t BitWidth, bool BigEndianTarget) {
- bool OnlyFullBytes = BitWidth % 8 == 0;
- unsigned NBytes = BitWidth / 8;
-
- size_t BitsHandled = 0;
- // Read all full bytes first
- for (size_t I = 0; I != NBytes; ++I) {
- std::byte B =
- BigEndianTarget ? data[NBytes - OnlyFullBytes - I] : data[I];
- for (unsigned X = 0; X != 8; ++X) {
- pushBit(bitof(B, X));
- ++BitsHandled;
- }
- }
-
- if (BitsHandled == BitWidth)
- return;
-
- // Rest of the bits.
- assert((BitWidth - BitsHandled) < 8);
- std::byte B = BigEndianTarget ? data[0] : data[NBytes];
- for (size_t I = 0, E = (BitWidth - BitsHandled); I != E; ++I) {
- pushBit(bitof(B, I));
- ++BitsHandled;
- }
-
- assert(BitsHandled == BitWidth);
- }
-};
-
/// We use this to recursively iterate over all fields and elemends of a pointer
/// and extract relevant data for a bitcast.
static bool enumerateData(const Pointer &P, const Context &Ctx, size_t Offset,
@@ -148,28 +82,28 @@ static bool enumerateData(const Pointer &P, const Context &Ctx, size_t Offset,
// Primitive arrays.
if (FieldDesc->isPrimitiveArray()) {
- bool BigEndianTarget = Ctx.getASTContext().getTargetInfo().isBigEndian();
QualType ElemType = FieldDesc->getElemQualType();
size_t ElemSizeInBits = Ctx.getASTContext().getTypeSize(ElemType);
PrimType ElemT = *Ctx.classify(ElemType);
// Special case, since the bools here are packed.
bool PackedBools = FieldDesc->getType()->isExtVectorBoolType();
+ unsigned NumElems = FieldDesc->getNumElems();
bool Ok = true;
- for (unsigned I = 0; I != FieldDesc->getNumElems(); ++I) {
- unsigned Index = BigEndianTarget ? (FieldDesc->getNumElems() - 1 - I) : I;
+ for (unsigned I = 0; I != NumElems; ++I) {
+ unsigned Index = I;
Ok = Ok && F(P.atIndex(Index), ElemT, Offset, PackedBools);
- Offset += ElemSizeInBits;
+ Offset += PackedBools ? 1 : ElemSizeInBits;
}
return Ok;
}
// Composite arrays.
if (FieldDesc->isCompositeArray()) {
- bool BigEndianTarget = Ctx.getASTContext().getTargetInfo().isBigEndian();
+ // bool BigEndianTarget = Ctx.getASTContext().getTargetInfo().isBigEndian();
QualType ElemType = FieldDesc->getElemQualType();
size_t ElemSizeInBits = Ctx.getASTContext().getTypeSize(ElemType);
for (unsigned I = 0; I != FieldDesc->getNumElems(); ++I) {
- unsigned Index = BigEndianTarget ? (FieldDesc->getNumElems() - 1 - I) : I;
+ unsigned Index = I;
enumerateData(P.atIndex(Index).narrow(), Ctx, Offset, F);
Offset += ElemSizeInBits;
}
@@ -178,7 +112,6 @@ static bool enumerateData(const Pointer &P, const Context &Ctx, size_t Offset,
// Records.
if (FieldDesc->isRecord()) {
- bool BigEndianTarget = Ctx.getASTContext().getTargetInfo().isBigEndian();
const Record *R = FieldDesc->ElemRecord;
const ASTRecordLayout &Layout =
Ctx.getASTContext().getASTRecordLayout(R->getDecl());
@@ -186,8 +119,7 @@ static bool enumerateData(const Pointer &P, const Context &Ctx, size_t Offset,
auto enumerateFields = [&]() -> void {
for (unsigned I = 0, N = R->getNumFields(); I != N; ++I) {
- const Record::Field *Fi =
- R->getField(BigEndianTarget ? (N - 1 - I) : I);
+ const Record::Field *Fi = R->getField(I);
Pointer Elem = P.atField(Fi->Offset);
size_t BitOffset =
Offset + Layout.getFieldOffset(Fi->Decl->getFieldIndex());
@@ -196,7 +128,7 @@ static bool enumerateData(const Pointer &P, const Context &Ctx, size_t Offset,
};
auto enumerateBases = [&]() -> void {
for (unsigned I = 0, N = R->getNumBases(); I != N; ++I) {
- const Record::Base *B = R->getBase(BigEndianTarget ? (N - 1 - I) : I);
+ const Record::Base *B = R->getBase(I);
Pointer Elem = P.atField(B->Offset);
CharUnits ByteOffset =
Layout.getBaseClassOffset(cast<CXXRecordDecl>(B->Decl));
@@ -204,14 +136,8 @@ static bool enumerateData(const Pointer &P, const Context &Ctx, size_t Offset,
Ok = Ok && enumerateData(Elem, Ctx, BitOffset, F);
}
};
-
- if (BigEndianTarget) {
- enumerateFields();
- enumerateBases();
- } else {
- enumerateBases();
- enumerateFields();
- }
+ enumerateBases();
+ enumerateFields();
return Ok;
}
@@ -295,26 +221,26 @@ static bool CheckBitcastType(InterpState &S, CodePtr OpPC, QualType T,
static bool readPointerToBuffer(const Context &Ctx, const Pointer &FromPtr,
BitcastBuffer &Buffer, bool ReturnOnUninit) {
const ASTContext &ASTCtx = Ctx.getASTContext();
- bool SwapData = (ASTCtx.getTargetInfo().isLittleEndian() !=
- llvm::sys::IsLittleEndianHost);
- bool BigEndianTarget = ASTCtx.getTargetInfo().isBigEndian();
+ Endian TargetEndianness =
+ ASTCtx.getTargetInfo().isLittleEndian() ? Endian::Little : Endian::Big;
return enumeratePointerFields(
FromPtr, Ctx,
[&](const Pointer &P, PrimType T, size_t BitOffset,
bool PackedBools) -> bool {
- if (!P.isInitialized()) {
- assert(false && "Implement uninitialized value tracking");
- return ReturnOnUninit;
- }
+ // if (!P.isInitialized()) {
+ // assert(false && "Implement uninitialized value tracking");
+ // return ReturnOnUninit;
+ // }
- assert(P.isInitialized());
+ // assert(P.isInitialized());
// nullptr_t is a PT_Ptr for us, but it's still not std::is_pointer_v.
if (T == PT_Ptr)
assert(false && "Implement casting to pointer types");
CharUnits ObjectReprChars = ASTCtx.getTypeSizeInChars(P.getType());
unsigned BitWidth = ASTCtx.toBits(ObjectReprChars);
+ unsigned FullSize = BitWidth;
llvm::SmallVector<std::byte> Buff(ObjectReprChars.getQuantity());
// Work around floating point types that contain unused padding bytes.
// This is really just `long double` on x86, which is the only
@@ -328,7 +254,7 @@ static bool readPointerToBuffer(const Context &Ctx, const Pointer &FromPtr,
F.bitcastToMemory(Buff.data());
// Now, only (maybe) swap the actual size of the float, excluding the
// padding bits.
- if (SwapData)
+ if (llvm::sys::IsBigEndianHost)
swapBytes(Buff.data(), NumBits / 8);
} else {
@@ -337,20 +263,15 @@ static bool readPointerToBuffer(const Context &Ctx, const Pointer &FromPtr,
else if (T == PT_Bool && PackedBools)
BitWidth = 1;
- BITCAST_TYPE_SWITCH(T, {
- T Val = P.deref<T>();
- Val.bitcastToMemory(Buff.data());
- });
- if (SwapData)
- swapBytes(Buff.data(), ObjectReprChars.getQuantity());
- }
+ BITCAST_TYPE_SWITCH(T,
+ { P.deref<T>().bitcastToMemory(Buff.data()); });
- if (BitWidth != (Buff.size() * 8) && BigEndianTarget) {
- Buffer.pushData(Buff.data() + (Buff.size() - 1 - (BitWidth / 8)),
- BitWidth, BigEndianTarget);
- } else {
- Buffer.pushData(Buff.data(), BitWidth, BigEndianTarget);
+ if (llvm::sys::IsBigEndianHost)
+ swapBytes(Buff.data(), FullSize / 8);
}
+
+ Buffer.pushData(Buff.data(), BitOffset, BitWidth, FullSize,
+ TargetEndianness);
return true;
});
}
@@ -362,7 +283,7 @@ bool clang::interp::DoBitCast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
assert(Ptr.isBlockPointer());
assert(Buff);
- BitcastBuffer Buffer;
+ BitcastBuffer Buffer(BuffSize * 8);
if (!CheckBitcastType(S, OpPC, Ptr.getType(), /*IsToType=*/false))
return false;
@@ -371,13 +292,20 @@ bool clang::interp::DoBitCast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
assert(Buffer.size() == BuffSize * 8);
HasIndeterminateBits = !Buffer.allInitialized();
- std::memcpy(Buff, Buffer.data(), BuffSize);
+
+ const ASTContext &ASTCtx = S.getASTContext();
+ Endian TargetEndianness =
+ ASTCtx.getTargetInfo().isLittleEndian() ? Endian::Little : Endian::Big;
+ auto B = Buffer.copyBits(0, BuffSize * 8, BuffSize * 8, TargetEndianness);
+
+ std::memcpy(Buff, B.get(), BuffSize);
if (llvm::sys::IsBigEndianHost)
swapBytes(Buff, BuffSize);
return Success;
}
+/// ---------------------------------------------------------------------------------------------------------------------
bool clang::interp::DoBitCastPtr(InterpState &S, CodePtr OpPC,
const Pointer &FromPtr, Pointer &ToPtr) {
@@ -394,43 +322,59 @@ bool clang::interp::DoBitCastPtr(InterpState &S, CodePtr OpPC,
if (!CheckBitcastType(S, OpPC, ToType, /*IsToType=*/true))
return false;
- BitcastBuffer Buffer;
+ const ASTContext &ASTCtx = S.getASTContext();
+
+ CharUnits ObjectReprChars = ASTCtx.getTypeSizeInChars(ToType);
+ BitcastBuffer Buffer(ASTCtx.toBits(ObjectReprChars));
readPointerToBuffer(S.getContext(), FromPtr, Buffer,
/*ReturnOnUninit=*/false);
// Now read the values out of the buffer again and into ToPtr.
- const ASTContext &ASTCtx = S.getASTContext();
- size_t BitOffset = 0;
+ Endian TargetEndianness =
+ ASTCtx.getTargetInfo().isLittleEndian() ? Endian::Little : Endian::Big;
bool Success = enumeratePointerFields(
ToPtr, S.getContext(),
- [&](const Pointer &P, PrimType T, size_t _, bool PackedBools) -> bool {
+ [&](const Pointer &P, PrimType T, size_t BitOffset,
+ bool PackedBools) -> bool {
+ CharUnits ObjectReprChars = ASTCtx.getTypeSizeInChars(P.getType());
+ unsigned FullBitWidth = ASTCtx.toBits(ObjectReprChars);
if (T == PT_Float) {
- CharUnits ObjectReprChars = ASTCtx.getTypeSizeInChars(P.getType());
const auto &Semantics = ASTCtx.getFloatTypeSemantics(P.getType());
unsigned NumBits = llvm::APFloatBase::getSizeInBits(Semantics);
assert(NumBits % 8 == 0);
assert(NumBits <= ASTCtx.toBits(ObjectReprChars));
- std::byte *M = Buffer.getBytes(BitOffset);
+ auto M = Buffer.copyBits(BitOffset, NumBits, FullBitWidth,
+ TargetEndianness);
if (llvm::sys::IsBigEndianHost)
- swapBytes(M, NumBits / 8);
+ swapBytes(M.get(), NumBits / 8);
- P.deref<Floating>() = Floating::bitcastFromMemory(M, Semantics);
+ P.deref<Floating>() = Floating::bitcastFromMemory(M.get(), Semantics);
P.initialize();
- BitOffset += ASTCtx.toBits(ObjectReprChars);
return true;
}
- BITCAST_TYPE_SWITCH_FIXED_SIZE(T, {
- std::byte *M = Buffer.getBytes(BitOffset);
+ unsigned BitWidth;
+ if (const FieldDecl *FD = P.getField(); FD && FD->isBitField())
+ BitWidth = FD->getBitWidthValue(ASTCtx);
+ else if (T == PT_Bool && PackedBools)
+ BitWidth = 1;
+ else
+ BitWidth = ASTCtx.toBits(ObjectReprChars);
- if (llvm::sys::IsBigEndianHost)
- swapBytes(M, T::bitWidth() / 8);
+ auto Memory = Buffer.copyBits(BitOffset, BitWidth, FullBitWidth,
+ TargetEndianness);
+ if (llvm::sys::IsBigEndianHost)
+ swapBytes(Memory.get(), FullBitWidth / 8);
- P.deref<T>() = T::bitcastFromMemory(M, T::bitWidth());
- P.initialize();
- BitOffset += T::bitWidth();
+ BITCAST_TYPE_SWITCH_FIXED_SIZE(T, {
+ if (BitWidth > 0)
+ P.deref<T>() = T::bitcastFromMemory(Memory.get(), T::bitWidth())
+ .truncate(BitWidth);
+ else
+ P.deref<T>() = T::zero();
});
+ P.initialize();
return true;
});
diff --git a/clang/test/AST/ByteCode/builtin-bit-cast-bitfields.cpp b/clang/test/AST/ByteCode/builtin-bit-cast-bitfields.cpp
new file mode 100644
index 00000000000000..084ec6e75a4472
--- /dev/null
+++ b/clang/test/AST/ByteCode/builtin-bit-cast-bitfields.cpp
@@ -0,0 +1,300 @@
+// RUN: %clang_cc1 -verify=expected,both -std=c++2a -fsyntax-only -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -verify=expected,both -std=c++2a -fsyntax-only -triple aarch64_be-linux-gnu -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -verify=expected,both -std=c++2a -fsyntax-only -fexperimental-new-constant-interpreter -triple powerpc64le-unknown-unknown -mabi=ieeelongdouble %s
+// RUN: %clang_cc1 -verify=expected,both -std=c++2a -fsyntax-only -fexperimental-new-constant-interpreter -triple powerpc64-unknown-unknown -mabi=ieeelongdouble %s
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+# define LITTLE_END 1
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+# define LITTLE_END 0
+#else
+# ...
[truncated]
|
tbaederr
force-pushed
the
builtin-bit-cast12
branch
7 times, most recently
from
5dde320 to
aac1f47
Compare
tbaederr
requested review from
zygoloid,
AaronBallman,
shafik,
erichkeane and
Sirraide
|
CI looks good and I've tested this on both LE and BE systems. Adding some reviewers to double-check. |
tbaederr
force-pushed
the
builtin-bit-cast12
branch
from
aac1f47 to
95c8d03
Compare
| else | ||
| DstBit = size() - BitOffset - BitWidth + It; | ||
|
|
||
| unsigned DstByte = (DstBit / 8); |
There was a problem hiding this comment.
Someday we're going to have to search the entire code base for the number 8, aren't we? :-D
|
|
||
| /// Track what bits have been initialized to known values and which ones | ||
| /// have indeterminate value. | ||
| /// All offsets are in bits. |
There was a problem hiding this comment.
There's a small part of me that wonders if we should use a different type for representing bits. e.g., instead of using size_t to mean byte offset and bit offset depending on context, I wonder if we want a BitOffset datatype (strong enum? constexpr class?) to represent bit offsets and have no implicit conversions to/from size_t to help avoid accidentally mixing bit and byte offsets. WDYT?
There was a problem hiding this comment.
Yeah I was wondering the same thing at some point, we could just have a POD struct wrapping a size_t or similar and use that for bits, we already have CharUnits for bytes although that's more "bytes for the target system", which might also be another distinction we have to make if the host and target byte size aren't the same... But I don't even want to think about that.
I can imagine just having
struct Bytes { size_t N; Bits toBits();}
struct Bits { size_t N; }and use those.
There was a problem hiding this comment.
There's a small part of me that wonders if we should use a different type for representing bits. e.g., instead of using
size_tto mean byte offset and bit offset depending on context,
I second that because I at least confuse those two all the time, to the point where all my compiler projects have a Size struct that requires you to always be explicit as to whether you’re passing in / want to have bits or bytes.
There was a problem hiding this comment.
Added something like this. In the end I almost didn't use the Bytes struct at all though.
| assert(false && "Implement uninitialized value tracking"); | ||
| return ReturnOnUninit; | ||
| } | ||
| // if (!P.isInitialized()) { |
There was a problem hiding this comment.
Kinda, it will be needed in the future but for now we can't assert here (anymore) since there's a different problem where we're not marking bases as initialized. not having the assert here enables us to do more testing.
There was a problem hiding this comment.
See the last commit in #117179
| static_assert(bit_cast<bits<2, unsigned char, pad>>(x) == 3); | ||
| static_assert( | ||
| bit_cast<X>((unsigned char)0x40).direction == X::direction::right); | ||
| } |
There was a problem hiding this comment.
Some more test coverage you might want to consider adding:
- Structs with zero-sized fields, like
struct S {}; struct T { S s; }; - Structs where the zero-sized field is
[[no_unique_address]]merged with other fields. - Structs with a flexible array member
There was a problem hiding this comment.
Those tests don't seem to be related to bitfields specifically, or are they?
There was a problem hiding this comment.
Whoops, brain fart, those are good tests in general but have nothing to do with bit-fields. You already have a zero-sized bit-field covered.
tbaederr
added a commit
to tbaederr/llvm-project
that referenced
this pull request
Nov 20, 2024
This looks a little ugly now, I should wait for llvm#116843.
tbaederr
force-pushed
the
builtin-bit-cast12
branch
7 times, most recently
from
b20305d to
7d58afa
Compare
|
Ping |
tbaederr
force-pushed
the
builtin-bit-cast12
branch
from
7d58afa to
df584d5
Compare
AaronBallman
approved these changes
Dec 3, 2024
tbaederr
force-pushed
the
builtin-bit-cast12
branch
from
df584d5 to
6e8a250
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/8464 Here is the relevant piece of the build log for the reference |
tbaederr
added a commit
that referenced
this pull request
Dec 4, 2024
This reverts commit 4b5e7fa. This breaks builders: https://lab.llvm.org/buildbot/#/builders/154/builds/8464 I guess some more testing on 32 bit hosts is needed.
tbaederr
added a commit
that referenced
this pull request
Dec 4, 2024
…)" This reverts commit 54db162. Check for existence of __SIZOEF_INT128__ so we don't run those tests on targets that don't have int128.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/39/builds/3195 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/38/builds/1110 Here is the relevant piece of the build log for the reference |
|
It looks like this still fails on i386 after the reapply: |
tbaederr
added a commit
that referenced
this pull request
Dec 5, 2024
This still breaks on 32bit hosts otherwise. See #116843
tbaederr
added a commit
to tbaederr/llvm-project
that referenced
this pull request
Dec 5, 2024
This looks a little ugly now, I should wait for llvm#116843.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/1632 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Copy the data one bit at a time, leaving optimizations for future work.
Adds a BitcastBuffer that takes care of pushing the bits in the right order.