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

[Daniel Dunbar]

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
>
>