[AIX] Check for typedef properly when getting preferred type align

[stevewan]

Summary

The current check for typedef is naive and doesn't deal with any convoluted cases. This patch makes use of the new 'AlignRequirement' enum field from 'TypeInfo' to determine whether or not this is an 'aligned' attribute on a typedef.

[rjmccall]

LGTM, except it's missing tests.

[stevewan]

In D109387#2987780, @rjmccall wrote:

LGTM, except it's missing tests.

Thanks. This is an NFC patch, the tests added in https://reviews.llvm.org/D107598 should still suffice IMHO.

[rjmccall]

Oh, no, I don't think this is NFC. It seems very likely that there's some case where the behavior is distinguishable. Maybe a typedef of a record with an alignment attribute?

__attribute__((aligned(2), packed)) struct float4 { float x, y, z, w; };
typedef struct float4 float4typedef;

struct {
  float4typedef field; // presumably still eligible for alignment upgrade
};

[stevewan]

In D109387#2987843, @rjmccall wrote:

Oh, no, I don't think this is NFC. It seems very likely that there's some case where the behavior is distinguishable. Maybe a typedef of a record with an alignment attribute?

__attribute__((aligned(2), packed)) struct float4 { float x, y, z, w; };
typedef struct float4 float4typedef;

struct {
  float4typedef field; // presumably still eligible for alignment upgrade
};

Right, the behaviour difference is exactly what I'm trying to fix, silly me :). Though the diff isn't very obvious in the given example because ABIAlign equals to PreferredAlign in that particular case, the following test may provide a better exposure,

struct __attribute__((aligned(2))) S { double x; };
typedef struct S SS;

[stevewan]

Add test case

[rjmccall]

Thanks, LGTM.

clang/lib/AST/ASTContext.cpp

unsigned ASTContext::getPreferredTypeAlign(const Type *T) const {
 
   if (!Target->allowsLargerPreferedTypeAlignment())
     return ABIAlign;
 
   if (const auto *RT = T->getAs<RecordType>()) {
     const RecordDecl *RD = RT->getDecl();
 
     // When used as part of a typedef, or together with a 'packed' attribute,
-    // the 'aligned' attribute can be used to decrease alignment.
-    if ((TI.isAlignRequired() && T->getAs<TypedefType>() != nullptr) ||
+    // the 'aligned' attribute can be used to decrease alignment. Note that the
+    // 'packed' case is already taken into consideration when computing the
+    // alignment, we only need to handle the typedef case here.
+    if (TI.AlignRequirement == AlignRequirementKind::RequiredByTypedef ||
         RD->isInvalidDecl())
       return ABIAlign;
 
     unsigned PreferredAlign = static_cast<unsigned>(
         toBits(getASTRecordLayout(RD).PreferredAlignment));
     assert(PreferredAlign >= ABIAlign &&
            "PreferredAlign should be at least as large as ABIAlign.");
     return PreferredAlign;

clang/test/Layout/aix-power-alignment-typedef-2.cpp

 // 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
 
-struct C {
+namespace test1 {
+struct S {
   double x;
 };
 
-typedef struct C __attribute__((__aligned__(2))) CC;
+typedef struct S __attribute__((__aligned__(2))) SS;
 
-CC cc;
+SS ss;
 
-// CHECK: @cc = global %struct.C zeroinitializer, align 2
+// CHECK: @{{.*}}test1{{.*}}ss{{.*}} = global %"struct.test1::S" zeroinitializer, align 2
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2))) S {
+  double x;
+};
+
+typedef struct S SS;
+
+SS ss;
+
+// CHECK: @{{.*}}test2{{.*}}ss{{.*}} = global %"struct.test2::S" zeroinitializer, align 8
+} // namespace test2