-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][NFC] Remove class layout scissor #89055
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
Conversation
@llvm/pr-subscribers-clang Author: Nathan Sidwell (urnathan) ChangesNow that Full diff: https://github.com/llvm/llvm-project/pull/89055.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..38167903cda508 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -75,7 +75,7 @@ struct CGRecordLowering {
// sentinel member type that ensures correct rounding.
struct MemberInfo {
CharUnits Offset;
- enum InfoKind { VFPtr, VBPtr, Field, Base, VBase, Scissor } Kind;
+ enum InfoKind { VFPtr, VBPtr, Field, Base, VBase } Kind;
llvm::Type *Data;
union {
const FieldDecl *FD;
@@ -197,7 +197,7 @@ struct CGRecordLowering {
const CXXRecordDecl *Query) const;
void calculateZeroInit();
CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
- void checkBitfieldClipping() const;
+ void checkBitfieldClipping(bool isNonVirtualBaseType) const;
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -299,8 +299,8 @@ void CGRecordLowering::lower(bool NVBaseType) {
accumulateVBases();
}
llvm::stable_sort(Members);
+ checkBitfieldClipping(NVBaseType);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- checkBitfieldClipping();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -894,8 +894,6 @@ CGRecordLowering::calculateTailClippingOffset(bool isNonVirtualBaseType) const {
}
void CGRecordLowering::accumulateVBases() {
- Members.push_back(MemberInfo(calculateTailClippingOffset(false),
- MemberInfo::Scissor, nullptr, RD));
for (const auto &Base : RD->vbases()) {
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
if (BaseDecl->isEmpty())
@@ -950,18 +948,20 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage representations.
-void CGRecordLowering::checkBitfieldClipping() const {
+void CGRecordLowering::checkBitfieldClipping(
+ bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
+ auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto &M : Members) {
- // Only members with data and the scissor can cut into tail padding.
- if (!M.Data && M.Kind != MemberInfo::Scissor)
+ // Only members with data could possibly overlap.
+ if (!M.Data)
continue;
assert(M.Offset >= Tail && "Bitfield access unit is not clipped");
- Tail = M.Offset;
- if (M.Data)
- Tail += getSize(M.Data);
+ Tail = M.Offset + getSize(M.Data);
+ assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) &&
+ "Bitfield straddles scissor offset");
}
#endif
}
|
@llvm/pr-subscribers-clang-codegen Author: Nathan Sidwell (urnathan) ChangesNow that Full diff: https://github.com/llvm/llvm-project/pull/89055.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..38167903cda508 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -75,7 +75,7 @@ struct CGRecordLowering {
// sentinel member type that ensures correct rounding.
struct MemberInfo {
CharUnits Offset;
- enum InfoKind { VFPtr, VBPtr, Field, Base, VBase, Scissor } Kind;
+ enum InfoKind { VFPtr, VBPtr, Field, Base, VBase } Kind;
llvm::Type *Data;
union {
const FieldDecl *FD;
@@ -197,7 +197,7 @@ struct CGRecordLowering {
const CXXRecordDecl *Query) const;
void calculateZeroInit();
CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
- void checkBitfieldClipping() const;
+ void checkBitfieldClipping(bool isNonVirtualBaseType) const;
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -299,8 +299,8 @@ void CGRecordLowering::lower(bool NVBaseType) {
accumulateVBases();
}
llvm::stable_sort(Members);
+ checkBitfieldClipping(NVBaseType);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- checkBitfieldClipping();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -894,8 +894,6 @@ CGRecordLowering::calculateTailClippingOffset(bool isNonVirtualBaseType) const {
}
void CGRecordLowering::accumulateVBases() {
- Members.push_back(MemberInfo(calculateTailClippingOffset(false),
- MemberInfo::Scissor, nullptr, RD));
for (const auto &Base : RD->vbases()) {
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
if (BaseDecl->isEmpty())
@@ -950,18 +948,20 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage representations.
-void CGRecordLowering::checkBitfieldClipping() const {
+void CGRecordLowering::checkBitfieldClipping(
+ bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
+ auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto &M : Members) {
- // Only members with data and the scissor can cut into tail padding.
- if (!M.Data && M.Kind != MemberInfo::Scissor)
+ // Only members with data could possibly overlap.
+ if (!M.Data)
continue;
assert(M.Offset >= Tail && "Bitfield access unit is not clipped");
- Tail = M.Offset;
- if (M.Data)
- Tail += getSize(M.Data);
+ Tail = M.Offset + getSize(M.Data);
+ assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) &&
+ "Bitfield straddles scissor offset");
}
#endif
}
|
ping? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be useful to mention #87090 in the commit message (first comment).
2d2dcde
to
4b93593
Compare
Now that
accumulateBitfields
does the correct clipping, we don't need the scissor any more --checkBitfieldClipping
can compute its location directly.