[PATCH] D18390: Fix logic for which symbols to keep with comdats
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 23 14:06:07 PDT 2016
Ok, that seems reasonable to me. Maybe add a FIXME comment unless it will
be fixed imminently.
On Wed, Mar 23, 2016 at 1:49 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> >> > ================
> >> > 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,
> I actually thing it is a bug, but would like to address that in a
> followup patch if possible.
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits