[PATCH] D18390: Fix logic for which symbols to keep with comdats

[Rafael Espíndola via llvm-commits]

>> > ================
>> > Comment at: lib/Linker/LinkModules.cpp:567
>> > @@ -567,2 +566,3 @@
>> >      for (GlobalValue *GV2 : ComdatMembers[SC])
>> > -      ValuesToLink.insert(GV2);
>> > +      if (GV2->hasInternalLinkage())
>> > +        ValuesToLink.insert(GV2);
>> > ----------------
>> > Why only internal members? If we have a comdat member in ValuesToLink (added in linkIfNeeded), then presumably we had selected that copy of the comdat and wouldn't we want all members?
>>
>> The correct thing is to drop this entire loop. I indent to do that in
>> a followup patch.
>
>
> I see now why we need to add internal members to the ValuesToLink set here - when we selected the dest copy of that particular symbol but need to keep a local (will be renamed) copy of the source symbol for references, right?

I actually thing it is a bug, but would like to address that in a
followup patch if possible.

Cheers,
Rafael