[patch] Support multiple comdat sections with the same name but different groups

David Blaikie dblaikie at gmail.com
Tue Oct 22 15:27:47 PDT 2013


On Sat, Oct 19, 2013 at 7:36 AM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> >> Can we use a DenseMap instead of a std::map?
> >
> >
> > Would require a bit more work - DenseMap can't handle a pair<string,
> string>
> > key (no tombstone value), not sure if there's something I can do to
> address
> > that.
>
> You would probably need to create dedicated class for the pair and
> implement densemapinfo for it. This is probably not too hot, lets keep
> the std::map for now and benchmark it once the rest of the patch is
> done.
>
> >>
> >> Can you add a new test and leave comdat.s unchanged? Would make it
> >> easier to see that the new code behaves the same in the case of
> >> distinct names.
> >
> >
> > I tend to prefer avoiding adding new test files if possible (keeping
> testing
> > related to the same feature together) to avoid excess process execution
> > overhead.
> >
> > It seems fairly easy to split out examples of different names and the
> same
> > name so they're all tested, but tested separately, in the same file.
> >
> > But I'm not wedded to the idea either.
>
> At least for now please add a second test. Object file dumps are
> somewhat unstable, which makes the test changes hard to read.
>

Attached an updated patch that has a separate test, removes the
RuntimeDyLib and another bit of noise in the change, and reverts the reloc
section ungrouping change.

Thanks,
- David


>
> A followup patch doing nothing but merging the tests would probably be OK.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131022/f4496070/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mc_group.diff
Type: application/octet-stream
Size: 4674 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131022/f4496070/attachment.obj>


More information about the llvm-commits mailing list