Fix the computation of alignment for fields of packed+aligned structs.

Part of the fix for PR8413.

llvm-svn: 123904

clang/include/clang/AST/Decl.h

@@ -1728,12 +1728,15 @@ public:
 class FieldDecl : public DeclaratorDecl {
   // FIXME: This can be packed into the bitfields in Decl.
   bool Mutable : 1;
+  mutable unsigned CachedFieldIndex : 31;
+
   Expr *BitWidth;
 protected:
   FieldDecl(Kind DK, DeclContext *DC, SourceLocation L,
             IdentifierInfo *Id, QualType T, TypeSourceInfo *TInfo,
             Expr *BW, bool Mutable)
-    : DeclaratorDecl(DK, DC, L, Id, T, TInfo), Mutable(Mutable), BitWidth(BW) {
+    : DeclaratorDecl(DK, DC, L, Id, T, TInfo),
+      Mutable(Mutable), CachedFieldIndex(0), BitWidth(BW) {
   }
 
 public:
@@ -1741,6 +1744,10 @@ public:
                            SourceLocation L, IdentifierInfo *Id, QualType T,
                            TypeSourceInfo *TInfo, Expr *BW, bool Mutable);
 
+  /// getFieldIndex - Returns the index of this field within its record,
+  /// as appropriate for passing to ASTRecordLayout::getFieldOffset.
+  unsigned getFieldIndex() const;
+
   /// isMutable - Determines whether this field is mutable (C++ only).
   bool isMutable() const { return Mutable; }
 

clang/lib/AST/ASTContext.cpp

@@ -590,8 +590,11 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool RefAsPointee) const {
     }
   }
 
+  // If we're using the align attribute only, just ignore everything
+  // else about the declaration and its type.
   if (UseAlignAttrOnly) {
-    // ignore type of value
+    // do nothing
+
   } else if (const ValueDecl *VD = dyn_cast<ValueDecl>(D)) {
     QualType T = VD->getType();
     if (const ReferenceType* RT = T->getAs<ReferenceType>()) {
@@ -617,11 +620,30 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool RefAsPointee) const {
       }
       Align = std::max(Align, getPreferredTypeAlign(T.getTypePtr()));
     }
-    if (const FieldDecl *FD = dyn_cast<FieldDecl>(VD)) {
-      // In the case of a field in a packed struct, we want the minimum
-      // of the alignment of the field and the alignment of the struct.
-      Align = std::min(Align,
-        getPreferredTypeAlign(FD->getParent()->getTypeForDecl()));
+
+    // Fields can be subject to extra alignment constraints, like if
+    // the field is packed, the struct is packed, or the struct has a
+    // a max-field-alignment constraint (#pragma pack).  So calculate
+    // the actual alignment of the field within the struct, and then
+    // (as we're expected to) constrain that by the alignment of the type.
+    if (const FieldDecl *field = dyn_cast<FieldDecl>(VD)) {
+      // So calculate the alignment of the field.
+      const ASTRecordLayout &layout = getASTRecordLayout(field->getParent());
+
+      // Start with the record's overall alignment.
+      unsigned fieldAlign = layout.getAlignment();
+
+      // Use the GCD of that and the offset within the record.
+      uint64_t offset = layout.getFieldOffset(field->getFieldIndex());
+      if (offset > 0) {
+        // Alignment is always a power of 2, so the GCD will be a power of 2,
+        // which means we get to do this crazy thing instead of Euclid's.
+        uint64_t lowBitOfOffset = offset & (~offset + 1);
+        if (lowBitOfOffset < fieldAlign)
+          fieldAlign = static_cast<unsigned>(lowBitOfOffset);
+      }
+
+      Align = std::min(Align, fieldAlign);
     }
   }
 

clang/lib/AST/Decl.cpp

@@ -1881,6 +1881,25 @@ bool FieldDecl::isAnonymousStructOrUnion() const {
   return false;
 }
 
+unsigned FieldDecl::getFieldIndex() const {
+  if (CachedFieldIndex) return CachedFieldIndex - 1;
+
+  unsigned index = 0;
+  RecordDecl::field_iterator
+    i = getParent()->field_begin(), e = getParent()->field_end();
+  while (true) {
+    assert(i != e && "failed to find field in parent!");
+    if (*i == this)
+      break;
+
+    ++i;
+    ++index;
+  }
+
+  CachedFieldIndex = index + 1;
+  return index;
+}
+
 //===----------------------------------------------------------------------===//
 // TagDecl Implementation
 //===----------------------------------------------------------------------===//

clang/lib/AST/ExprConstant.cpp

@@ -1565,14 +1565,7 @@ bool IntExprEvaluator::VisitOffsetOfExpr(const OffsetOfExpr *E) {
         return false;
       RecordDecl *RD = RT->getDecl();
       const ASTRecordLayout &RL = Info.Ctx.getASTRecordLayout(RD);
-      unsigned i = 0;
-      // FIXME: It would be nice if we didn't have to loop here!
-      for (RecordDecl::field_iterator Field = RD->field_begin(),
-                                      FieldEnd = RD->field_end();
-           Field != FieldEnd; (void)++Field, ++i) {
-        if (*Field == MemberDecl)
-          break;
-      }
+      unsigned i = MemberDecl->getFieldIndex();
       assert(i < RL.getFieldCount() && "offsetof field in wrong type");
       Result += Info.Ctx.toCharUnitsFromBits(RL.getFieldOffset(i));
       CurrentType = MemberDecl->getType().getNonReferenceType();

clang/test/CodeGen/packed-structure.c

@@ -87,3 +87,16 @@ int s2_load_y(struct s2 *a) { return a->y; }
 // CHECK-FUNCTIONS: define void @s2_copy
 // CHECK-FUNCTIONS: call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}}, i8* {{.*}}, i64 8, i32 2, i1 false)
 void s2_copy(struct s2 *a, struct s2 *b) { *b = *a; }
+
+struct __attribute__((packed, aligned)) s3 {
+  short aShort;
+  int anInt;
+};
+// CHECK-GLOBAL: @s3_1 = global i32 2
+int s3_1 = __alignof(((struct s3*) 0)->anInt);
+// CHECK-FUNCTIONS: define i32 @test3(
+int test3(struct s3 *ptr) {
+  // CHECK-FUNCTIONS:      [[PTR:%.*]] = getelementptr inbounds {{%.*}}* {{%.*}}, i32 0, i32 1
+  // CHECK-FUNCTIONS-NEXT: load i32* [[PTR]], align 2
+  return ptr->anInt;
+}