[clang] 731abdf - [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

Vladislav Dzhidzhoev via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 15 04:12:31 PST 2022


Author: Vladislav Dzhidzhoev
Date: 2022-12-15T15:10:41+03:00
New Revision: 731abdfdcc33d813e6c3b4b89eff307aa5c18083

URL: https://github.com/llvm/llvm-project/commit/731abdfdcc33d813e6c3b4b89eff307aa5c18083
DIFF: https://github.com/llvm/llvm-project/commit/731abdfdcc33d813e6c3b4b89eff307aa5c18083.diff

LOG: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

RecordLayoutBuilder assumes the size of a potentially-overlapping field
with non-zero size as the size of the base subobject type corresponding
to the field type.
Make CGRecordLayoutBuilder to acknowledge that in order to avoid incorrect
padding insertion.

Differential Revision: https://reviews.llvm.org/D139741

Added: 
    clang/test/CodeGenCXX/no-unique-address-3.cpp

Modified: 
    clang/include/clang/AST/Decl.h
    clang/lib/AST/Decl.cpp
    clang/lib/AST/RecordLayoutBuilder.cpp
    clang/lib/CodeGen/CGRecordLayoutBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index cd33fcef56619..268ec7deeb814 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3062,6 +3062,10 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
   /// [[no_unique_address]] attribute.
   bool isZeroSize(const ASTContext &Ctx) const;
 
+  /// Determine if this field is of potentially-overlapping class type, that
+  /// is, subobject with the [[no_unique_address]] attribute
+  bool isPotentiallyOverlapping() const;
+
   /// Get the kind of (C++11) default member initializer that this field has.
   InClassInitStyle getInClassInitStyle() const {
     InitStorageKind storageKind = InitStorage.getInt();

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index b1fdc897bf27e..0f15d7b3b6d33 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4339,6 +4339,10 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
   return true;
 }
 
+bool FieldDecl::isPotentiallyOverlapping() const {
+  return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
+}
+
 unsigned FieldDecl::getFieldIndex() const {
   const FieldDecl *Canonical = getCanonicalDecl();
   if (Canonical != this)

diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 52bd3f20221f4..56cdbb75dd4b2 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1853,9 +1853,8 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
 void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
                                              bool InsertExtraPadding) {
   auto *FieldClass = D->getType()->getAsCXXRecordDecl();
-  bool PotentiallyOverlapping = D->hasAttr<NoUniqueAddressAttr>() && FieldClass;
   bool IsOverlappingEmptyField =
-      PotentiallyOverlapping && FieldClass->isEmpty();
+      D->isPotentiallyOverlapping() && FieldClass->isEmpty();
 
   CharUnits FieldOffset =
       (IsUnion || IsOverlappingEmptyField) ? CharUnits::Zero() : getDataSize();
@@ -1916,7 +1915,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
 
     // A potentially-overlapping field occupies its dsize or nvsize, whichever
     // is larger.
-    if (PotentiallyOverlapping) {
+    if (D->isPotentiallyOverlapping()) {
       const ASTRecordLayout &Layout = Context.getASTRecordLayout(FieldClass);
       EffectiveFieldSize =
           std::max(Layout.getNonVirtualSize(), Layout.getDataSize());

diff  --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 6f85bca8a2013..0f2d764bc9498 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -379,9 +379,14 @@ void CGRecordLowering::accumulateFields() {
       for (++Field; Field != FieldEnd && Field->isBitField(); ++Field);
       accumulateBitFields(Start, Field);
     } else if (!Field->isZeroSize(Context)) {
+      // Use base subobject layout for the potentially-overlapping field,
+      // as it is done in RecordLayoutBuilder
       Members.push_back(MemberInfo(
           bitsToCharUnits(getFieldBitOffset(*Field)), MemberInfo::Field,
-          getStorageType(*Field), *Field));
+          Field->isPotentiallyOverlapping()
+              ? getStorageType(Field->getType()->getAsCXXRecordDecl())
+              : getStorageType(*Field),
+          *Field));
       ++Field;
     } else {
       ++Field;

diff  --git a/clang/test/CodeGenCXX/no-unique-address-3.cpp b/clang/test/CodeGenCXX/no-unique-address-3.cpp
new file mode 100644
index 0000000000000..f21e9d1d2b01e
--- /dev/null
+++ b/clang/test/CodeGenCXX/no-unique-address-3.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fdump-record-layouts -std=c++17 %s -o %t | FileCheck %s
+
+// CHECK:       *** Dumping AST Record Layout
+// CHECK-NEXT:           0 | class Empty (empty)
+// CHECK-NEXT:             | [sizeof=1, dsize=1, align=1,
+// CHECK-NEXT:             |  nvsize=1, nvalign=1]
+// CHECK:       *** Dumping AST Record Layout
+// CHECK-NEXT:           0 | class Second
+// CHECK-NEXT:           0 |   class Empty (base) (empty)
+// CHECK-NEXT:       0:0-0 |   short A
+// CHECK-NEXT:             | [sizeof=2, dsize=1, align=2,
+// CHECK-NEXT:             |  nvsize=1, nvalign=2]
+// CHECK:       *** Dumping AST Record Layout
+// CHECK-NEXT:           0 | class Foo
+// CHECK-NEXT:           0 |   class Empty (base) (empty)
+// CHECK-NEXT:           2 |   class Second NZNoUnique
+// CHECK-NEXT:           2 |     class Empty (base) (empty)
+// CHECK-NEXT:       2:0-0 |     short A
+// CHECK-NEXT:           3 |   char B
+// CHECK-NEXT:             | [sizeof=4, dsize=4, align=2,
+// CHECK-NEXT:             |  nvsize=4, nvalign=2]
+
+class Empty {};
+
+// CHECK-LABEL: LLVMType:%class.Second = type { i8, i8 }
+// CHECK-NEXT:  NonVirtualBaseLLVMType:%class.Second.base = type { i8 }
+class Second : Empty {
+  short A : 1;
+};
+
+// CHECK-LABEL:   LLVMType:%class.Foo = type { [2 x i8], %class.Second.base, i8 }
+// CHECK-NEXT:    NonVirtualBaseLLVMType:%class.Foo = type { [2 x i8], %class.Second.base, i8 }
+class Foo : Empty {
+  [[no_unique_address]] Second NZNoUnique;
+  char B;
+};
+Foo I;
+
+// CHECK:       *** Dumping AST Record Layout
+// CHECK-NEXT:           0 | class SecondEmpty (empty)
+// CHECK-NEXT:           0 |   class Empty (base) (empty)
+// CHECK-NEXT:             | [sizeof=1, dsize=0, align=1,
+// CHECK-NEXT:             |  nvsize=1, nvalign=1]
+class SecondEmpty: Empty {
+};
+
+// CHECK:       *** Dumping AST Record Layout
+// CHECK-NEXT:           0 | class Bar
+// CHECK-NEXT:           0 |   class Empty (base) (empty)
+// CHECK-NEXT:           1 |   class SecondEmpty ZNoUnique (empty)
+// CHECK-NEXT:           1 |     class Empty (base) (empty)
+// CHECK-NEXT:           0 |   char C
+// CHECK-NEXT:             | [sizeof=2, dsize=1, align=1,
+// CHECK-NEXT:             |  nvsize=2, nvalign=1]
+
+// CHECK-LABEL:  LLVMType:%class.Bar = type { i8, i8 }
+// CHECK-NEXT:   NonVirtualBaseLLVMType:%class.Bar = type { i8, i8 }
+class Bar : Empty {
+  [[no_unique_address]] SecondEmpty ZNoUnique;
+  char C;
+};
+Bar J;
+
+// CHECK:       *** Dumping AST Record Layout
+// CHECK-NEXT:           0 | class IntFieldClass
+// CHECK-NEXT:           0 |   class Empty (base) (empty)
+// CHECK-NEXT:           2 |   class Second Field
+// CHECK-NEXT:           2 |     class Empty (base) (empty)
+// CHECK-NEXT:       2:0-0 |     short A
+// CHECK-NEXT:           4 |   int C
+// CHECK-NEXT:             | [sizeof=8, dsize=8, align=4,
+// CHECK-NEXT:             |  nvsize=8, nvalign=4]
+
+// CHECK-LABEL:   LLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second.base, i32 }
+// CHECK-NEXT:    NonVirtualBaseLLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second.base, i32 }
+class IntFieldClass : Empty {
+  [[no_unique_address]] Second Field;
+  int C;
+};
+IntFieldClass K;


        


More information about the cfe-commits mailing list