[cfe-commits] r66481 - in /cfe/trunk: lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/objc-align.m

[Fariborz Jahanian]

ObjC sections have generally symbols of the same kind. Those which has  
all pointers should be
aligned on a pointer alignment. Hard-coding to 4 makes the alignment  
incorrect if we ever want to
support let's say old abi in 64-bit mode. I think we should get the  
alignment from the type of
the initializers which go into the section. And if we don't yet have  
the initializer, set the alignment
when initializers become available.

- fariborz

On Mar 9, 2009, at 3:41 PM, Daniel Dunbar wrote:

> I'm not sure what you mean by getting the alignment of the symbol that
> this section is for.
>
> I don't like hard coding the alignment either, but the only "right"
> way to get the correct alignment without hardcoding would be to make
> up the C types for the structures that these sections are supposed to
> contain and then use our existing machinery to get the alignment.
> Getting the alignment of, say, a pointer type feels worse to me; it is
> still hard coding, since it isn't actually using the right type, and
> it also isn't really encoding the correct dependency (although maybe
> it is a little more robust).
>
> Lets discuss in person if you disagree... I could be swayed. :)
>
> - Daniel
>
> On Mon, Mar 9, 2009 at 3:28 PM, Fariborz Jahanian  
> <fjahanian at apple.com> wrote:
>>
>> On Mar 9, 2009, at 3:18 PM, Daniel Dunbar wrote:
>>
>>> Author: ddunbar
>>> Date: Mon Mar  9 17:18:41 2009
>>> New Revision: 66481
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=66481&view=rev
>>> Log:
>>> NeXT: Set alignment on a number of ObjC metadata variables (matching
>>> llvm-gcc).
>>>
>>> Added:
>>>   cfe/trunk/test/CodeGenObjC/objc-align.m
>>> Modified:
>>>   cfe/trunk/lib/CodeGen/CGObjCMac.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=66481&r1=66480&r2=66481&view=diff
>>>
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Mon Mar  9 17:18:41 2009
>>> @@ -1086,6 +1086,7 @@
>>>
>>> std::string("\01L_OBJC_PROTOCOL_")+ProtocolName,
>>>                               &CGM.getModule());
>>>    Entry->setSection("__OBJC,__protocol,regular,no_dead_strip");
>>> +    Entry->setAlignment(4);
>>>    UsedGlobals.push_back(Entry);
>>>    // FIXME: Is this necessary? Why only for protocol?
>>>    Entry->setAlignment(4);
>>> @@ -1108,6 +1109,7 @@
>>>                               "\01L_OBJC_PROTOCOL_" +
>>> PD->getNameAsString(),
>>>                               &CGM.getModule());
>>>    Entry->setSection("__OBJC,__protocol,regular,no_dead_strip");
>>> +    Entry->setAlignment(4);
>>>    UsedGlobals.push_back(Entry);
>>>    // FIXME: Is this necessary? Why only for protocol?
>>>    Entry->setAlignment(4);
>>> @@ -1195,7 +1197,7 @@
>>>  llvm::Constant *Init = llvm::ConstantStruct::get(Values);
>>>  llvm::GlobalVariable *GV =
>>>    CreateMetadataVar(Name, Init,
>>> "__OBJC,__cat_cls_meth,regular,no_dead_strip",
>>> -                      0, false);
>>> +                      4, false);
>>>  return llvm::ConstantExpr::getBitCast(GV,  
>>> ObjCTypes.ProtocolListPtrTy);
>>> }
>>>
>>> @@ -1354,7 +1356,7 @@
>>>  llvm::GlobalVariable *GV =
>>>    CreateMetadataVar(std::string("\01L_OBJC_CATEGORY_")+ExtName,  
>>> Init,
>>>                      "__OBJC,__category,regular,no_dead_strip",
>>> -                      0, true);
>>> +                      4, true);
>>>  DefinedCategories.push_back(GV);
>>> }
>>>
>>> @@ -1489,8 +1491,7 @@
>>>  llvm::GlobalVariable *GV =
>>>    CreateMetadataVar(std::string("\01L_OBJC_CLASS_")+ClassName,  
>>> Init,
>>>                      "__OBJC,__class,regular,no_dead_strip",
>>> -                      32, // FIXME: Why?
>>> -                      true);
>>> +                      4, true);
>>>  DefinedClasses.push_back(GV);
>>> }
>>>
>>> @@ -1559,9 +1560,8 @@
>>>                                  &CGM.getModule());
>>>  }
>>>  GV->setSection("__OBJC,__meta_class,regular,no_dead_strip");
>>> +  GV->setAlignment(4);
>>>  UsedGlobals.push_back(GV);
>>> -  // FIXME: Why?
>>> -  GV->setAlignment(32);
>>>
>>>  return GV;
>>> }
>>> @@ -1715,8 +1715,8 @@
>>>  llvm::GlobalVariable *GV;
>>>  if (ForClass)
>>>    GV = CreateMetadataVar("\01L_OBJC_CLASS_VARIABLES_" +
>>> ID->getNameAsString(),
>>> -                           Init,
>>> "__OBJC,__cls_vars,regular,no_dead_strip",
>>> -                           32, true);
>>> +                           Init,
>>> "__OBJC,__class_vars,regular,no_dead_strip",
>>> +                           4, true);
>>>  else
>>>    GV = CreateMetadataVar("\01L_OBJC_INSTANCE_VARIABLES_"
>>>                           + ID->getNameAsString(),
>>> @@ -2349,7 +2349,7 @@
>>>  CreateMetadataVar("\01L_OBJC_MODULES",
>>>                    llvm::ConstantStruct::get(ObjCTypes.ModuleTy,  
>>> Values),
>>>                    "__OBJC,__module_info,regular,no_dead_strip",
>>> -                    0, true);
>>> +                    4, true);
>>> }
>>>
>>> llvm::Constant *CGObjCMac::EmitModuleSymbols() {
>>> @@ -3493,8 +3493,7 @@
>>>                               Init,
>>>                               "\01L_OBJC_LABEL_CLASS_$",
>>>                               &CGM.getModule());
>>> -    GV->setAlignment(
>>> -       
>>> CGM.getTargetData().getPrefTypeAlignment(ObjCTypes.Int8PtrTy));
>>> +    GV->setAlignment(8);
>>
>> Why are we hard coding the alignment here and in other places,  
>> instead of
>> getting the alignment of the symbol that this
>> section is for?
>>
>> - fariborz
>>
>>