[patch][pr19848] Don't create new comdats in CodeGen

[Rafael Espíndola]

Updated patch attached. Just waiting on the review of the clang one.

On 8 January 2015 at 13:14, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2015-Jan-08, at 08:33, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>
>> On 8 January 2015 at 03:30, David Majnemer <david.majnemer at gmail.com> wrote:
>>> I really like this change.  However, don't we need to upgrade existing
>>> bitcode by inserting stuff into newly made comdats?
>>
>> I guess so.
>>
>> It was not initially clear to me that it would be worth it. Old
>> bitcode files would still work, they would just produce .o files that
>> would not be as optimized by ELF/COFF linkers.
>>
>> But the feature is common enough that we should probably upgrade. The
>> attached patch does that.
>>
>> Cheers,
>> Rafael
>> <t.patch>
>
> Aside from a comment nit, this LGTM.  I agree that upgrading makes sense.
>
>> diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
>> index 7c7eebd..c9c08c0 100644
>> --- a/lib/Bitcode/Reader/BitcodeReader.cpp
>> +++ b/lib/Bitcode/Reader/BitcodeReader.cpp
>> @@ -100,19 +100,31 @@ static bool ConvertToString(ArrayRef<uint64_t> Record, unsigned Idx,
>>    return false;
>>  }
>>
>> +static bool hasImplicitComdat(size_t Val) {
>> +  switch (Val) {
>> +  default:
>> +    return false;
>> +  case 1:  // old WeakAnyLinkage
>> +  case 4:  // old LinkOnceAnyLinkage
>> +  case 10: // old WeakODRLinkage
>> +  case 11: // old LinkOnceODRLinkage
>> +    return true;
>> +  }
>> +}
>> +
>>  static GlobalValue::LinkageTypes getDecodedLinkage(unsigned Val) {
>>    switch (Val) {
>>    default: // Map unknown/new linkages to external
>>    case 0:
>>      return GlobalValue::ExternalLinkage;
>>    case 1:
>> -    return GlobalValue::WeakAnyLinkage;
>> +    return GlobalValue::WeakAnyLinkage; // Old value
>
> I think `// Implicit comdat` is better than `// Old value` here.  Or
> maybe `// Old value with implicit comdat`?
>
>>    case 2:
>>      return GlobalValue::AppendingLinkage;
>>    case 3:
>>      return GlobalValue::InternalLinkage;
>>    case 4:
>> -    return GlobalValue::LinkOnceAnyLinkage;
>> +    return GlobalValue::LinkOnceAnyLinkage; // Old value
>>    case 5:
>>      return GlobalValue::ExternalLinkage; // Obsolete DLLImportLinkage
>>    case 6:
>> @@ -124,9 +136,9 @@ static GlobalValue::LinkageTypes getDecodedLinkage(unsigned Val) {
>>    case 9:
>>      return GlobalValue::PrivateLinkage;
>>    case 10:
>> -    return GlobalValue::WeakODRLinkage;
>> +    return GlobalValue::WeakODRLinkage; // Old value
>>    case 11:
>> -    return GlobalValue::LinkOnceODRLinkage;
>> +    return GlobalValue::LinkOnceODRLinkage; // Old value
>>    case 12:
>>      return GlobalValue::AvailableExternallyLinkage;
>>    case 13:
>>
>> @@ -135,6 +147,14 @@ static GlobalValue::LinkageTypes getDecodedLinkage(unsigned Val) {
>>      return GlobalValue::PrivateLinkage; // Obsolete LinkerPrivateWeakLinkage
>>    case 15:
>>      return GlobalValue::ExternalLinkage; // Obsolete LinkOnceODRAutoHideLinkage
>> +  case 16:
>> +    return GlobalValue::WeakAnyLinkage;
>
> Or maybe you should just move it down here:
>
>     case 1: // Old value with implicit comdat
>     case 16:
>         return GlobalValue::WeakAnyLinkage;
>
>> +  case 17:
>> +    return GlobalValue::WeakODRLinkage;
>> +  case 18:
>> +    return GlobalValue::LinkOnceAnyLinkage;
>> +  case 19:
>> +    return GlobalValue::LinkOnceODRLinkage;
>>    }
>>  }
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 20145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150108/0baf2e05/attachment.bin>