[cfe-commits] r69771 - in /cfe/trunk: lib/AST/ASTContext.cpp lib/AST/DeclObjC.cpp lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/ivar-layout-64.m

Daniel Dunbar daniel at zuster.org
Tue Apr 21 20:45:12 PDT 2009


Author: ddunbar
Date: Tue Apr 21 22:45:12 2009
New Revision: 69771

URL: http://llvm.org/viewvc/llvm-project?rev=69771&view=rev
Log:
Rework the shadow struct that is layed out for Objective-C classes.

 - Superclasses are now always laid out their shadow structure at the
   first field.

 - Prior to this, the entire class heirarchy was flattened into a
   single structure which meant that alignment, padding, and bitfields
   weren't packed correctly (the ASTRecordLayout was correct however,
   which meant our debug info didn't coincide with ivar offsets, for
   example).

 - This is still very suboptimal, but I believe the ivar layout itself
   is now at least close to correct.

 - <rdar://problem/6773388> error: objc[29823]: layout bitmap sliding
   backwards

Added:
    cfe/trunk/test/CodeGenObjC/ivar-layout-64.m
Modified:
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/DeclObjC.cpp
    cfe/trunk/lib/CodeGen/CGObjCMac.cpp

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

==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Apr 21 22:45:12 2009
@@ -621,11 +621,9 @@
   Alignment = std::max(Alignment, FieldAlign);
 }
 
-void ASTContext::CollectObjCIvars(const ObjCInterfaceDecl *OI,
-                             llvm::SmallVectorImpl<FieldDecl*> &Fields) {
-  const ObjCInterfaceDecl *SuperClass = OI->getSuperClass();
-  if (SuperClass)
-    CollectObjCIvars(SuperClass, Fields);
+static void CollectLocalObjCIvars(ASTContext *Ctx,
+                                  const ObjCInterfaceDecl *OI,
+                                  llvm::SmallVectorImpl<FieldDecl*> &Fields) {
   for (ObjCInterfaceDecl::ivar_iterator I = OI->ivar_begin(),
        E = OI->ivar_end(); I != E; ++I) {
     ObjCIvarDecl *IVDecl = *I;
@@ -633,20 +631,28 @@
       Fields.push_back(cast<FieldDecl>(IVDecl));
   }
   // look into properties.
-  for (ObjCInterfaceDecl::prop_iterator I = OI->prop_begin(*this),
-       E = OI->prop_end(*this); I != E; ++I) {
+  for (ObjCInterfaceDecl::prop_iterator I = OI->prop_begin(*Ctx),
+       E = OI->prop_end(*Ctx); I != E; ++I) {
     if (ObjCIvarDecl *IV = (*I)->getPropertyIvarDecl())
       Fields.push_back(cast<FieldDecl>(IV));
   }
 }
 
+
+void ASTContext::CollectObjCIvars(const ObjCInterfaceDecl *OI,
+                             llvm::SmallVectorImpl<FieldDecl*> &Fields) {
+  if (const ObjCInterfaceDecl *SuperClass = OI->getSuperClass())
+    CollectObjCIvars(SuperClass, Fields);
+  CollectLocalObjCIvars(this, OI, Fields);
+}
+
 /// addRecordToClass - produces record info. for the class for its
 /// ivars and all those inherited.
 ///
 const RecordDecl *ASTContext::addRecordToClass(const ObjCInterfaceDecl *D) {
   // FIXME: The only client relying on this working in the presence of
-  // forward declarations is IRgen, which should not need it. Fix
-  // and simplify this code.
+  // forward declarations is CodeGenTypes in IRgen, which should not
+  // need it. Fix and simplify this code.
   RecordDecl *&RD = ASTRecordForInterface[D];
   if (RD) {
     // If we have a record decl already and it is either a definition or if 'D'
@@ -662,11 +668,28 @@
                                    D->getIdentifier());
   
   llvm::SmallVector<FieldDecl*, 32> RecFields;
-  CollectObjCIvars(D, RecFields);
+  CollectLocalObjCIvars(this, D, RecFields);
   
   if (RD == 0)
     RD = RecordDecl::Create(*this, TagDecl::TK_struct, 0, D->getLocation(),
                             D->getIdentifier());
+
+  
+  const RecordDecl *SRD;
+  if (const ObjCInterfaceDecl *SuperClass = D->getSuperClass()) {
+    SRD = addRecordToClass(SuperClass);
+  } else {
+    SRD = RecordDecl::Create(*this, TagDecl::TK_struct, 0, SourceLocation(), 0);
+    const_cast<RecordDecl*>(SRD)->completeDefinition(*this);
+  }
+
+  RD->addDecl(*this, 
+              FieldDecl::Create(*this, RD,
+                                SourceLocation(),
+                                0,
+                                getTagDeclType(const_cast<RecordDecl*>(SRD)),
+                                0, false));
+
   /// FIXME! Can do collection of ivars and adding to the record while
   /// doing it.
   for (unsigned i = 0, e = RecFields.size(); i != e; ++i) {

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

==============================================================================
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Tue Apr 21 22:45:12 2009
@@ -380,14 +380,32 @@
 ///
 const FieldDecl *
 ObjCInterfaceDecl::lookupFieldDeclForIvar(ASTContext &Context, 
-                                          const ObjCIvarDecl *IVar) const {
+                                          const ObjCIvarDecl *OIVD) const {
   assert(!isForwardDecl() && "Invalid interface decl!");
   const RecordDecl *RecordForDecl = Context.addRecordToClass(this);
-  assert(RecordForDecl && "lookupFieldDeclForIvar no storage for class");
   DeclContext::lookup_const_result Lookup =
-    RecordForDecl->lookup(Context, IVar->getDeclName());
-  assert((Lookup.first != Lookup.second) && "field decl not found");
-  return cast<FieldDecl>(*Lookup.first);
+    RecordForDecl->lookup(Context, OIVD->getDeclName());
+
+  if (Lookup.first != Lookup.second)
+    return cast<FieldDecl>(*Lookup.first);
+
+  // If lookup failed, try the superclass.
+  //
+  // FIXME: This is very non-performant. However, the root problem
+  // here is not the lookup itself. The main issue is that we should
+  // be able to map from an IvarDecl back to the context it lives
+  // inside; then this problem goes away. Currently, however,
+  // IvarDecl's live inside the translation unit!!!!
+  //
+  // Fixing IvarDecl's is less obvious than it might appear, we need
+  // to choose where synthesized ivars should live, and we also need
+  // to decide where to put IvarDecl's which appeared in an
+  // implementation context (either in the situation where they must
+  // duplicate the instance variables, or if there was no instance
+  // declaration).
+  const ObjCInterfaceDecl *OID = getSuperClass();
+  assert(OID && "field decl not found!");
+  return OID->lookupFieldDeclForIvar(Context, OIVD);
 }
 
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=69771&r1=69770&r2=69771&view=diff

==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Tue Apr 21 22:45:12 2009
@@ -4220,62 +4220,29 @@
   return GV;
 }
 
-/// countInheritedIvars - count number of ivars in class and its super class(s)
-///
-static int countInheritedIvars(const ObjCInterfaceDecl *OI, 
-                               ASTContext &Context) {
-  int count = 0;
-  if (!OI)
-    return 0;
-  const ObjCInterfaceDecl *SuperClass = OI->getSuperClass();
-  if (SuperClass)
-    count += countInheritedIvars(SuperClass, Context);
-  for (ObjCInterfaceDecl::ivar_iterator I = OI->ivar_begin(),
-       E = OI->ivar_end(); I != E; ++I)
-    ++count;
-  // look into properties.
-  for (ObjCInterfaceDecl::prop_iterator I = OI->prop_begin(Context),
-       E = OI->prop_end(Context); I != E; ++I) {
-    if ((*I)->getPropertyIvarDecl())
-      ++count;
-  }
-  return count;
-}
-
 void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCInterfaceDecl *OID,
                                               uint32_t &InstanceStart,
                                               uint32_t &InstanceSize) {
   assert(!OID->isForwardDecl() && "Invalid interface decl!");
   const llvm::StructLayout *Layout = GetInterfaceDeclStructLayout(OID);
     
-  int countSuperClassIvars = countInheritedIvars(OID->getSuperClass(),
-                                                 CGM.getContext());
   const RecordDecl *RD = CGM.getContext().addRecordToClass(OID);
-  RecordDecl::field_iterator firstField = RD->field_begin(CGM.getContext());
-  RecordDecl::field_iterator lastField = RD->field_end(CGM.getContext());
-  while (countSuperClassIvars-- > 0) {
-    lastField = firstField;
-    ++firstField;
-  }
-    
-  for (RecordDecl::field_iterator e = RD->field_end(CGM.getContext()),
-         ifield = firstField; ifield != e; ++ifield)
-    lastField = ifield;
-    
-  InstanceStart = InstanceSize = 0;
-  if (lastField != RD->field_end(CGM.getContext())) {
-    FieldDecl *Field = *lastField;
-    const llvm::Type *FieldTy =
-      CGM.getTypes().ConvertTypeForMem(Field->getType());
-    unsigned Size = CGM.getTargetData().getTypePaddedSize(FieldTy);
-    InstanceSize = GetIvarBaseOffset(Layout, Field) + Size;
-    if (firstField == RD->field_end(CGM.getContext()))
-      InstanceStart = InstanceSize;
-    else {
-      Field = *firstField;
-      InstanceStart =  GetIvarBaseOffset(Layout, Field);
-    }
-  }
+  
+  // Field 0 is always the superclass record (which may be empty).
+  RecordDecl::field_iterator fi = RD->field_begin(CGM.getContext());
+  RecordDecl::field_iterator fe = RD->field_end(CGM.getContext());
+  assert(fi != fe && "class record should never be empty!");
+
+  InstanceStart = CGM.getContext().getTypeSize((*fi)->getType()) / 8;
+
+  // We report the size of the instance as the offset following the
+  // last ivar (which is, of course, not the actual size).
+  std::advance(fi, std::distance(fi, fe) - 1);
+  const FieldDecl *LastField = *fi;
+  uint64_t Offset = GetIvarBaseOffset(Layout, LastField);
+  // Add size of type (rounded to next byte).
+  InstanceSize = (Offset + 
+                  (CGM.getContext().getTypeSize(LastField->getType()) + 7) / 8);
 }
 
 void CGObjCNonFragileABIMac::GenerateClass(const ObjCImplementationDecl *ID) {

Added: cfe/trunk/test/CodeGenObjC/ivar-layout-64.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/ivar-layout-64.m?rev=69771&view=auto

==============================================================================
--- cfe/trunk/test/CodeGenObjC/ivar-layout-64.m (added)
+++ cfe/trunk/test/CodeGenObjC/ivar-layout-64.m Tue Apr 21 22:45:12 2009
@@ -0,0 +1,56 @@
+// RUN: clang-cc -triple x86_64-apple-darwin9 -emit-llvm -o %t %s &&
+// RUNX: llvm-gcc -m64 -emit-llvm -S -o %t %s &&
+
+// RUN: grep '@"OBJC_IVAR_$_I3._iv2" = global i64 8, section "__DATA, __objc_const", align 8' %t &&
+// RUN: grep '@"OBJC_IVAR_$_I3._iv3" = global i64 12, section "__DATA, __objc_const", align 8' %t &&
+// RUN: grep '@"OBJC_IVAR_$_I4._iv4" = global i64 16, section "__DATA, __objc_const", align 8' %t &&
+// RUN: grep '@"OBJC_IVAR_$_I5._iv5" = global i64 24, section "__DATA, __objc_const", align 8' %t &&
+// RUN: grep '@"OBJC_IVAR_$_I5._iv6_synth" = global i64 28, section "__DATA, __objc_const", align 8' %t &&
+// RUN: grep '@"OBJC_IVAR_$_I5._iv7_synth" = global i64 32, section "__DATA, __objc_const", align 8' %t &&
+
+// RUN: true
+
+struct s0 {
+  double x;
+};
+
+ at interface I2 {
+  struct s0 _iv1;
+}
+ at end
+
+ at interface I3 : I2 {
+  unsigned int _iv2 :1;
+  unsigned : 0;
+  unsigned int _iv3 : 3;
+}
+ at end
+
+ at interface I4 : I3 {
+ char _iv4;
+}
+ at end
+
+ at interface I5 : I4 {
+ char _iv5;
+}
+
+ at property int prop0;
+ at end
+
+ at implementation I3
+ at end
+
+ at implementation I4 
+ at end
+
+ at interface I5 ()
+ at property int prop1;
+ at property char prop2;
+ at end
+
+ at implementation I5
+ at synthesize prop0 = _iv6_synth;
+ at synthesize prop1 = _iv7_synth;
+ at synthesize prop2 = _iv5;
+ at end





More information about the cfe-commits mailing list