[cfe-commits] r51762 - in /cfe/trunk: lib/AST/ASTContext.cpp test/Sema/bitfield-layout.c test/Sema/struct-packed-align.c

Eli Friedman eli.friedman at gmail.com
Fri May 30 02:31:39 PDT 2008


Author: efriedma
Date: Fri May 30 04:31:38 2008
New Revision: 51762

URL: http://llvm.org/viewvc/llvm-project?rev=51762&view=rev
Log:
Cleanup/refactoring of Sema struct layout.  This patch unifies the struct
and union codepaths and fixes some minor bugs.

I'm reasonably confident this is accurate, at least for X86.  I'll 
correct any bugs as I find them; I haven't found any for a while, 
though.


Added:
    cfe/trunk/test/Sema/bitfield-layout.c
Modified:
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/test/Sema/struct-packed-align.c

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=51762&r1=51761&r2=51762&view=diff

==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri May 30 04:31:38 2008
@@ -210,8 +210,8 @@
     std::pair<uint64_t, unsigned> EltInfo = 
       getTypeInfo(cast<VectorType>(T)->getElementType());
     Width = EltInfo.first*cast<VectorType>(T)->getNumElements();
-    // FIXME: Vector alignment is not the alignment of its elements.
-    Align = EltInfo.second;
+    // FIXME: This isn't right for unusual vectors
+    Align = Width;
     break;
   }
 
@@ -316,121 +316,102 @@
 /// position information.
 const ASTRecordLayout &ASTContext::getASTRecordLayout(const RecordDecl *D) {
   assert(D->isDefinition() && "Cannot get layout of forward declarations!");
-  
+
   // Look up this layout, if already laid out, return what we have.
   const ASTRecordLayout *&Entry = ASTRecordLayouts[D];
   if (Entry) return *Entry;
-  
+
   // Allocate and assign into ASTRecordLayouts here.  The "Entry" reference can
   // be invalidated (dangle) if the ASTRecordLayouts hashtable is inserted into.
   ASTRecordLayout *NewEntry = new ASTRecordLayout();
   Entry = NewEntry;
-  
+
   uint64_t *FieldOffsets = new uint64_t[D->getNumMembers()];
   uint64_t RecordSize = 0;
   unsigned RecordAlign = 8;  // Default alignment = 1 byte = 8 bits.
+  bool StructIsPacked = D->getAttr<PackedAttr>();
+  bool IsUnion = (D->getKind() == Decl::Union);
 
-  if (D->getKind() != Decl::Union) {
-    if (const AlignedAttr *AA = D->getAttr<AlignedAttr>())
-      RecordAlign = std::max(RecordAlign, AA->getAlignment());
-        
-    bool StructIsPacked = D->getAttr<PackedAttr>();
-    
-    // Layout each field, for now, just sequentially, respecting alignment.  In
-    // the future, this will need to be tweakable by targets.
-    for (unsigned i = 0, e = D->getNumMembers(); i != e; ++i) {
-      const FieldDecl *FD = D->getMember(i);
-      bool FieldIsPacked = StructIsPacked || FD->getAttr<PackedAttr>();
-      uint64_t FieldSize;
-      unsigned FieldAlign;
-      
-      if (const Expr *BitWidthExpr = FD->getBitWidth()) {
-        llvm::APSInt I(32);
-        bool BitWidthIsICE = 
-          BitWidthExpr->isIntegerConstantExpr(I, *this);
-        assert (BitWidthIsICE  && "Invalid BitField size expression");
-        FieldSize = I.getZExtValue();
+  if (const AlignedAttr *AA = D->getAttr<AlignedAttr>())
+    RecordAlign = std::max(RecordAlign, AA->getAlignment());
 
-        std::pair<uint64_t, unsigned> TypeInfo = getTypeInfo(FD->getType());
-        uint64_t TypeSize = TypeInfo.first;
-        
-        if (const AlignedAttr *AA = FD->getAttr<AlignedAttr>())
-          FieldAlign = AA->getAlignment();
-        else if (FieldIsPacked)
-          FieldAlign = 8;
-        else {
-          FieldAlign = TypeInfo.second;
-        }
-
-        // Check if we need to add padding to give the field the correct
-        // alignment.
-        if (RecordSize % FieldAlign + FieldSize > TypeSize)
-          RecordSize = (RecordSize+FieldAlign-1) & ~(FieldAlign-1);
+  // Layout each field, for now, just sequentially, respecting alignment.  In
+  // the future, this will need to be tweakable by targets.
+  for (unsigned i = 0, e = D->getNumMembers(); i != e; ++i) {
+    const FieldDecl *FD = D->getMember(i);
+    bool FieldIsPacked = StructIsPacked || FD->getAttr<PackedAttr>();
+    uint64_t FieldOffset = IsUnion ? 0 : RecordSize;
+    uint64_t FieldSize;
+    unsigned FieldAlign;
+
+    if (const Expr *BitWidthExpr = FD->getBitWidth()) {
+      // TODO: Need to check this algorithm on other targets!
+      //       (tested on Linux-X86)
+      llvm::APSInt I(32);
+      bool BitWidthIsICE = 
+        BitWidthExpr->isIntegerConstantExpr(I, *this);
+      assert (BitWidthIsICE  && "Invalid BitField size expression");
+      FieldSize = I.getZExtValue();
 
-      } else {
-        if (FD->getType()->isIncompleteType()) {
-          // This must be a flexible array member; we can't directly
-          // query getTypeInfo about these, so we figure it out here.
-          // Flexible array members don't have any size, but they
-          // have to be aligned appropriately for their element type.
-        
-          if (const AlignedAttr *AA = FD->getAttr<AlignedAttr>())
-            FieldAlign = AA->getAlignment();
-          else if (FieldIsPacked)
-            FieldAlign = 8;
-          else {
-            const ArrayType* ATy = FD->getType()->getAsArrayType();
-            FieldAlign = getTypeAlign(ATy->getElementType());
-          }
-          FieldSize = 0;
-        } else {
-          std::pair<uint64_t, unsigned> FieldInfo = getTypeInfo(FD->getType());
-          FieldSize = FieldInfo.first;
-        
-          if (const AlignedAttr *AA = FD->getAttr<AlignedAttr>())
-            FieldAlign = AA->getAlignment();
-          else if (FieldIsPacked)
-            FieldAlign = 8;
-          else
-            FieldAlign = FieldInfo.second;
-        }
+      std::pair<uint64_t, unsigned> FieldInfo = getTypeInfo(FD->getType());
+      uint64_t TypeSize = FieldInfo.first;
 
-        // Round up the current record size to the field's alignment boundary.
-        RecordSize = (RecordSize+FieldAlign-1) & ~(FieldAlign-1);
+      FieldAlign = FieldInfo.second;
+      if (FieldIsPacked)
+        FieldAlign = 1;
+      if (const AlignedAttr *AA = FD->getAttr<AlignedAttr>())
+        FieldAlign = std::max(FieldAlign, AA->getAlignment());
+
+      // Check if we need to add padding to give the field the correct
+      // alignment.
+      if (FieldSize == 0 || (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize)
+        FieldOffset = (FieldOffset + (FieldAlign-1)) & ~(FieldAlign-1);
+
+      // Padding members don't affect overall alignment
+      if (!FD->getIdentifier())
+        FieldAlign = 1;
+    } else {
+      if (FD->getType()->isIncompleteType()) {
+        // This must be a flexible array member; we can't directly
+        // query getTypeInfo about these, so we figure it out here.
+        // Flexible array members don't have any size, but they
+        // have to be aligned appropriately for their element type.
+        FieldSize = 0;
+        const ArrayType* ATy = FD->getType()->getAsArrayType();
+        FieldAlign = getTypeAlign(ATy->getElementType());
+      } else {
+        std::pair<uint64_t, unsigned> FieldInfo = getTypeInfo(FD->getType());
+        FieldSize = FieldInfo.first;
+        FieldAlign = FieldInfo.second;
       }
-      
-      // Place this field at the current location.
-      FieldOffsets[i] = RecordSize;
-      
-      // Reserve space for this field.
-      RecordSize += FieldSize;
-      
-      // Remember max struct/class alignment.
-      RecordAlign = std::max(RecordAlign, FieldAlign);
-    }
-    
-    // Finally, round the size of the total struct up to the alignment of the
-    // struct itself.
-    RecordSize = (RecordSize+RecordAlign-1) & ~(RecordAlign-1);
-  } else {
-    // Union layout just puts each member at the start of the record.
-    for (unsigned i = 0, e = D->getNumMembers(); i != e; ++i) {
-      const FieldDecl *FD = D->getMember(i);
-      std::pair<uint64_t, unsigned> FieldInfo = getTypeInfo(FD->getType());
-      uint64_t FieldSize = FieldInfo.first;
-      unsigned FieldAlign = FieldInfo.second;
+
+      if (FieldIsPacked)
+        FieldAlign = 8;
+      if (const AlignedAttr *AA = FD->getAttr<AlignedAttr>())
+        FieldAlign = std::max(FieldAlign, AA->getAlignment());
 
       // Round up the current record size to the field's alignment boundary.
-      RecordSize = std::max(RecordSize, FieldSize);
+      FieldOffset = (FieldOffset + (FieldAlign-1)) & ~(FieldAlign-1);
+    }
 
-      // Place this field at the start of the record.
-      FieldOffsets[i] = 0;
+    // Place this field at the current location.
+    FieldOffsets[i] = FieldOffset;
 
-      // Remember max struct/class alignment.
-      RecordAlign = std::max(RecordAlign, FieldAlign);
+    // Reserve space for this field.
+    if (IsUnion) {
+      RecordSize = std::max(RecordSize, FieldSize);
+    } else {
+      RecordSize = FieldOffset + FieldSize;
     }
+
+    // Remember max struct/class alignment.
+    RecordAlign = std::max(RecordAlign, FieldAlign);
   }
-  
+
+  // Finally, round the size of the total struct up to the alignment of the
+  // struct itself.
+  RecordSize = (RecordSize + (RecordAlign-1)) & ~(RecordAlign-1);
+
   NewEntry->SetLayout(RecordSize, RecordAlign, FieldOffsets);
   return *NewEntry;
 }

Added: cfe/trunk/test/Sema/bitfield-layout.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/bitfield-layout.c?rev=51762&view=auto

==============================================================================
--- cfe/trunk/test/Sema/bitfield-layout.c (added)
+++ cfe/trunk/test/Sema/bitfield-layout.c Fri May 30 04:31:38 2008
@@ -0,0 +1,32 @@
+// RUN: clang %s -fsyntax-only -verify -triple=i686-apple-darwin9
+
+#define CHECK_SIZE(kind, name, size) extern int name##1[sizeof(kind name) == size ? 1 : -1];
+#define CHECK_ALIGN(kind, name, size) extern int name##2[__alignof(kind name) == size ? 1 : -1];
+
+// Zero-width bit-fields
+struct a {char x; int : 0; char y;};
+CHECK_SIZE(struct, a, 5)
+CHECK_ALIGN(struct, a, 1)
+
+union b {char x; int : 0; char y;};
+CHECK_SIZE(union, b, 1)
+CHECK_ALIGN(union, b, 1)
+
+// Unnamed bit-field align
+struct c {char x; int : 20;};
+CHECK_SIZE(struct, c, 4)
+CHECK_ALIGN(struct, c, 1)
+
+union d {char x; int : 20;};
+CHECK_SIZE(union, d, 3)
+CHECK_ALIGN(union, d, 1)
+
+// Bit-field packing
+struct __attribute__((packed)) e {int x : 4, y : 30, z : 30;};
+CHECK_SIZE(struct, e, 8)
+CHECK_ALIGN(struct, e, 1)
+
+// Alignment on bit-fields
+struct f {__attribute((aligned(8))) int x : 30, y : 30, z : 30;};
+CHECK_SIZE(struct, f, 24)
+CHECK_ALIGN(struct, f, 8)

Modified: cfe/trunk/test/Sema/struct-packed-align.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/struct-packed-align.c?rev=51762&r1=51761&r2=51762&view=diff

==============================================================================
--- cfe/trunk/test/Sema/struct-packed-align.c (original)
+++ cfe/trunk/test/Sema/struct-packed-align.c Fri May 30 04:31:38 2008
@@ -69,3 +69,23 @@
   int ted_likes_cheese;
   void *args[] __attribute__((packed));
 };
+
+// Packed union
+union __attribute__((packed)) au4 {char c; int x;};
+extern int h1[sizeof(union au4) == 4 ? 1 : -1];
+extern int h2[__alignof(union au4) == 1 ? 1 : -1];
+
+// Aligned union
+union au5 {__attribute__((aligned(4))) char c;};
+extern int h1[sizeof(union au5) == 4 ? 1 : -1];
+extern int h2[__alignof(union au5) == 4 ? 1 : -1];
+
+// Alignment+packed
+struct as6 {char c; __attribute__((packed, aligned(2))) int x;};
+extern int i1[sizeof(struct as6) == 6 ? 1 : -1];
+extern int i2[__alignof(struct as6) == 2 ? 1 : -1];
+
+union au6 {char c; __attribute__((packed, aligned(2))) int x;};
+extern int k1[sizeof(union au6) == 4 ? 1 : -1];
+extern int k2[__alignof(union au6) == 2 ? 1 : -1];
+





More information about the cfe-commits mailing list