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

[Teresa Johnson via llvm-commits]

Ok, that seems reasonable to me. Maybe add a FIXME comment unless it will
be fixed imminently.
Teresa

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,
> right?
>
> I actually thing it is a bug, but would like to address that in a
> followup patch if possible.
>
> Cheers,
> Rafael
>



-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160323/b55fcfcf/attachment-0001.html>