[patch] Support multiple comdat sections with the same name but different groups
Rafael EspĂndola
rafael.espindola at gmail.com
Sat Oct 19 07:36:02 PDT 2013
>> 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.
A followup patch doing nothing but merging the tests would probably be OK.
Cheers,
Rafael
More information about the llvm-commits
mailing list