[clang] 71b170c - [AIX] "aligned" attribute does not decrease alignment

Steven Wan via cfe-commits cfe-commits at lists.llvm.org
Sun Aug 29 18:33:34 PDT 2021


Author: Steven Wan
Date: 2021-08-29T21:33:05-04:00
New Revision: 71b170ccf36ee02e6a4c472bc1d3e89bbaf0e2b4

URL: https://github.com/llvm/llvm-project/commit/71b170ccf36ee02e6a4c472bc1d3e89bbaf0e2b4
DIFF: https://github.com/llvm/llvm-project/commit/71b170ccf36ee02e6a4c472bc1d3e89bbaf0e2b4.diff

LOG: [AIX] "aligned" attribute does not decrease alignment

The "aligned" attribute can only increase the alignment of a struct, or struct member, unless it's used together with the "packed" attribute, or used as a part of a typedef, in which case, the "aligned" attribute can both increase and decrease alignment.

That said, we expect:
1. "aligned" attribute alone: does not interfere with the alignment upgrade instrumented by the AIX "power" alignment rule,
2. "aligned" attribute + typedef: overrides any computed alignment,
3. "aligned" attribute + "packed" attribute: overrides any computed alignment.
The old implementation achieved 2 and 3, but didn't get 1 right, in that any field marked attribute "aligned" would not go through the alignment upgrade.

Reviewed By: rjmccall

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

Added: 
    clang/test/Layout/aix-type-align-and-pack-attr.cpp

Modified: 
    clang/lib/AST/RecordLayoutBuilder.cpp
    clang/test/Layout/aix-power-alignment-typedef.cpp

Removed: 
    clang/test/Layout/aix-alignof-align-and-pack-attr.cpp


################################################################################
diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 30e88da9ac4f5..5c86b06da835e 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1968,6 +1968,19 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
     }
   }
 
+  // When used as part of a typedef, or together with a 'packed' attribute, the
+  // 'aligned' attribute can be used to decrease alignment. In that case, it
+  // overrides any computed alignment we have, and there is no need to upgrade
+  // the alignment.
+  auto alignedAttrCanDecreaseAIXAlignment = [AlignRequirement, FieldPacked] {
+    // Enum alignment sources can be safely ignored here, because this only
+    // helps decide whether we need the AIX alignment upgrade, which only
+    // applies to floating-point types.
+    return AlignRequirement == AlignRequirementKind::RequiredByTypedef ||
+           (AlignRequirement == AlignRequirementKind::RequiredByRecord &&
+            FieldPacked);
+  };
+
   // The AIX `power` alignment rules apply the natural alignment of the
   // "first member" if it is of a floating-point data type (or is an aggregate
   // whose recursively "first" member or element is such a type). The alignment
@@ -1978,8 +1991,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
   // and zero-width bit-fields count as prior members; members of empty class
   // types marked `no_unique_address` are not considered to be prior members.
   CharUnits PreferredAlign = FieldAlign;
-  if (DefaultsToAIXPowerAlignment &&
-      AlignRequirement == AlignRequirementKind::None &&
+  if (DefaultsToAIXPowerAlignment && !alignedAttrCanDecreaseAIXAlignment() &&
       (FoundFirstNonOverlappingEmptyFieldForAIX || IsNaturalAlign)) {
     auto performBuiltinTypeAlignmentUpgrade = [&](const BuiltinType *BTy) {
       if (BTy->getKind() == BuiltinType::Double ||
@@ -1990,12 +2002,13 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
       }
     };
 
-    const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
-    if (const ComplexType *CTy = Ty->getAs<ComplexType>()) {
-      performBuiltinTypeAlignmentUpgrade(CTy->getElementType()->castAs<BuiltinType>());
-    } else if (const BuiltinType *BTy = Ty->getAs<BuiltinType>()) {
+    const Type *BaseTy = D->getType()->getBaseElementTypeUnsafe();
+    if (const ComplexType *CTy = BaseTy->getAs<ComplexType>()) {
+      performBuiltinTypeAlignmentUpgrade(
+          CTy->getElementType()->castAs<BuiltinType>());
+    } else if (const BuiltinType *BTy = BaseTy->getAs<BuiltinType>()) {
       performBuiltinTypeAlignmentUpgrade(BTy);
-    } else if (const RecordType *RT = Ty->getAs<RecordType>()) {
+    } else if (const RecordType *RT = BaseTy->getAs<RecordType>()) {
       const RecordDecl *RD = RT->getDecl();
       assert(RD && "Expected non-null RecordDecl.");
       const ASTRecordLayout &FieldRecord = Context.getASTRecordLayout(RD);

diff  --git a/clang/test/Layout/aix-alignof-align-and-pack-attr.cpp b/clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
deleted file mode 100644
index 51f3c5a2adc11..0000000000000
--- a/clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
-// RUN:   FileCheck %s
-
-// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
-// RUN:   FileCheck %s
-
-namespace test1 {
-struct __attribute__((__aligned__(2))) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
-} // namespace test1
-
-namespace test2 {
-struct __attribute__((__aligned__(2), packed)) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
-} // namespace test2
-
-namespace test3 {
-struct __attribute__((__aligned__(16))) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test3{{.*}}c{{.*}} = global %"struct.test3::C" zeroinitializer, align 16
-} // namespace test3

diff  --git a/clang/test/Layout/aix-power-alignment-typedef.cpp b/clang/test/Layout/aix-power-alignment-typedef.cpp
index fc973a1fdfd81..f6925c34b1712 100644
--- a/clang/test/Layout/aix-power-alignment-typedef.cpp
+++ b/clang/test/Layout/aix-power-alignment-typedef.cpp
@@ -37,3 +37,39 @@ int x = sizeof(U);
 // CHECK-NEXT:       |  nvsize=2, nvalign=2, preferrednvalign=2]
 
 } // namespace test2
+
+namespace test3 {
+typedef double DblArr[] __attribute__((__aligned__(2)));
+
+union U {
+  DblArr da;
+  char x;
+};
+
+int x = sizeof(U);
+
+// CHECK:          0 | union test3::U
+// CHECK-NEXT:     0 |   test3::DblArr da
+// CHECK-NEXT:     0 |   char x
+// CHECK-NEXT:       | [sizeof=2, dsize=2, align=2, preferredalign=2,
+// CHECK-NEXT:       |  nvsize=2, nvalign=2, preferrednvalign=2]
+
+} // namespace test3
+
+namespace test4 {
+typedef double Dbl __attribute__((__aligned__(2)));
+
+union U {
+  Dbl DblArr[];
+  char x;
+};
+
+int x = sizeof(U);
+
+// CHECK:          0 | union test4::U
+// CHECK-NEXT:     0 |   test4::Dbl [] DblArr
+// CHECK-NEXT:     0 |   char x
+// CHECK-NEXT:       | [sizeof=2, dsize=2, align=2, preferredalign=2,
+// CHECK-NEXT:       |  nvsize=2, nvalign=2, preferrednvalign=2]
+
+} // namespace test4

diff  --git a/clang/test/Layout/aix-type-align-and-pack-attr.cpp b/clang/test/Layout/aix-type-align-and-pack-attr.cpp
new file mode 100644
index 0000000000000..d119511b141f2
--- /dev/null
+++ b/clang/test/Layout/aix-type-align-and-pack-attr.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test1{{.*}}s{{.*}} = global %"struct.test1::S" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test2{{.*}}s{{.*}} = global %"struct.test2::S" zeroinitializer, align 2
+} // namespace test2
+
+namespace test3 {
+struct __attribute__((__aligned__(16))) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test3{{.*}}s{{.*}} = global %"struct.test3::S" zeroinitializer, align 16
+} // namespace test3
+
+namespace test4 {
+struct __attribute__((aligned(2))) SS {
+  double d;
+};
+
+struct S {
+  struct SS ss;
+} s;
+
+// CHECK: @{{.*}}test4{{.*}}s{{.*}} = global %"struct.test4::S" zeroinitializer, align 8
+} // namespace test4
+
+namespace test5 {
+struct __attribute__((aligned(2), packed)) SS {
+  double d;
+};
+
+struct S {
+  struct SS ss;
+} s;
+
+// CHECK: @{{.*}}test5{{.*}}s{{.*}} = global %"struct.test5::S" zeroinitializer, align 2
+} // namespace test5


        


More information about the cfe-commits mailing list