[PATCH] D18390: Fix logic for which symbols to keep with comdats
Rafael Espíndola via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 23 13:49:15 PDT 2016
>> > ================
>> > 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.
More information about the llvm-commits