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

[Duncan P. N. Exon Smith]

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