[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-12 Thread Nathan Sidwell via cfe-commits 

https://github.com/urnathan closed 
https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-10 Thread Nathan Sidwell via cfe-commits 

https://github.com/urnathan updated 
https://github.com/llvm/llvm-project/pull/89055

>From 9ab483f3451bfcaa7968c5f1cf7115144522f58a Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH 1/3] [clang] Remove class layout scissor

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048..cc51cc3476c43 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  : 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  : 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
 }

>From 050df411c74bdab8d9d6562c127abc92babbb527 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Wed, 17 Apr 2024 17:15:57 -0400
Subject: [PATCH 2/3] Fix param spelling

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index cc51cc3476c43..38167903cda50 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -949,9 +949,9 @@ void CGRecordLowering::calculateZeroInit() {
 
 // Verify accumulateBitfields computed the correct storage representations.
 void CGRecordLowering::checkBitfieldClipping(
-bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
+bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
 #ifndef NDEBUG
-  auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
+  auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
   auto Tail = CharUnits::Zero();
   for (const auto  : Members) {
 // Only members with data could possibly overlap.

>From 4b93593a63850f4165979a38018d6c4be23dd681 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Mon, 6 May 2024 10:28:06 -0400
Subject: [PATCH 3/3] lose attribute

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 38167903cda50..5169be204c14d 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -948,8 +948,7 @@ void CGRecordLowering::calculateZeroInit() {
 }
 
 // Verify accumulateBitfields computed the correct storage

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-06 Thread Nathan Sidwell via cfe-commits 

https://github.com/urnathan updated 
https://github.com/llvm/llvm-project/pull/89055

>From db5e6456f26ea9b859d3ff24161d7494d58bb7e1 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH 1/3] [clang] Remove class layout scissor

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..cc51cc3476c438 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  : 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  : 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
 }

>From 36705e5bcdcda6983ed5a163ae8ea3e9911ad275 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Wed, 17 Apr 2024 17:15:57 -0400
Subject: [PATCH 2/3] Fix param spelling

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index cc51cc3476c438..38167903cda508 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -949,9 +949,9 @@ void CGRecordLowering::calculateZeroInit() {
 
 // Verify accumulateBitfields computed the correct storage representations.
 void CGRecordLowering::checkBitfieldClipping(
-bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
+bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
 #ifndef NDEBUG
-  auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
+  auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
   auto Tail = CharUnits::Zero();
   for (const auto  : Members) {
 // Only members with data could possibly overlap.

>From 2d2dcdecb0328b8d397bb14072e5750ddf20a39a Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Mon, 6 May 2024 10:28:06 -0400
Subject: [PATCH 3/3] lose attribute

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 38167903cda508..5169be204c14d0 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -948,8 +948,7 @@ void CGRecordLowering::calculateZeroInit() {
 }
 
 // Verify accumulateBitfields computed the correct storage

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-04 Thread Fangrui Song via cfe-commits 

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-04 Thread Fangrui Song via cfe-commits 

https://github.com/MaskRay approved this pull request.

Might be useful to mention #87090 in the commit message (first comment).

https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-04 Thread Fangrui Song via cfe-commits 


@@ -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 {

MaskRay wrote:

The doc of LLVM_ATTRIBUTE_UNUSED doesn't suggest variable uses.

Since we use `-Wno-unused-parameter` and `IsNonVirtualBaseType` is used in 
!NDEBUG code paths, we can just use `(bool IsNonVirtualBaseType)`

https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-05-03 Thread Nathan Sidwell via cfe-commits 

urnathan wrote:

ping?

https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-04-17 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Nathan Sidwell (urnathan)


Changes

Now that `accumulateBitfields` does the correct clipping, we don't need the 
scissor any more -- `checkBitfieldClipping` can compute its location directly.

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


1 Files Affected:

- (modified) clang/lib/CodeGen/CGRecordLayoutBuilder.cpp (+11-11) 


``diff
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  : 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  : 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
 }

``




https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-04-17 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: Nathan Sidwell (urnathan)


Changes

Now that `accumulateBitfields` does the correct clipping, we don't need the 
scissor any more -- `checkBitfieldClipping` can compute its location directly.

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


1 Files Affected:

- (modified) clang/lib/CodeGen/CGRecordLayoutBuilder.cpp (+11-11) 


``diff
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  : 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  : 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
 }

``




https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-04-17 Thread Nathan Sidwell via cfe-commits 

https://github.com/urnathan ready_for_review 
https://github.com/llvm/llvm-project/pull/89055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-04-17 Thread Nathan Sidwell via cfe-commits 

https://github.com/urnathan updated 
https://github.com/llvm/llvm-project/pull/89055

>From db5e6456f26ea9b859d3ff24161d7494d58bb7e1 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH 1/2] [clang] Remove class layout scissor

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..cc51cc3476c438 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  : 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  : 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
 }

>From 36705e5bcdcda6983ed5a163ae8ea3e9911ad275 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Wed, 17 Apr 2024 17:15:57 -0400
Subject: [PATCH 2/2] Fix param spelling

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index cc51cc3476c438..38167903cda508 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -949,9 +949,9 @@ void CGRecordLowering::calculateZeroInit() {
 
 // Verify accumulateBitfields computed the correct storage representations.
 void CGRecordLowering::checkBitfieldClipping(
-bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
+bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
 #ifndef NDEBUG
-  auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
+  auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
   auto Tail = CharUnits::Zero();
   for (const auto  : Members) {
 // Only members with data could possibly overlap.

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Remove class layout scissor (PR #89055)

2024-04-17 Thread Nathan Sidwell via cfe-commits 

https://github.com/urnathan created 
https://github.com/llvm/llvm-project/pull/89055

Now that `accumulateBitfields` does the correct clipping, we don't need the 
scissor any more -- `checkBitfieldClipping` can compute its location directly.

>From db5e6456f26ea9b859d3ff24161d7494d58bb7e1 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell 
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH] [clang] Remove class layout scissor

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..cc51cc3476c438 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  : 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  : 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
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits