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

[Daniel Dunbar]

We discussed this offline and decided to just force all metadata
variables to pointer alignment for know. This is something of a hack
but ensures that things should be packed tightly while not penalizing
codegen.

 - Daniel

On Mon, Mar 9, 2009 at 3:49 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> 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
>>>
>>>
>
>