[PATCH] D31735: ThinLTOBitcodeWriter: delete comdats if their keys are renamed

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D31735#719700, @pcc wrote:

> It isn't clear whether this does the right thing if the comdat contains associated members, only some of which need to be moved to the merged module. Although I believe that the frontend will never attach !type metadata to a global in a comdat with associated members, the asan globals feature could add an associated member to such a comdat.
>
> An optimal fix would be to rename the comdat to match the symbol name and ensure that all comdat members are moved to the merged module if any member needs to be moved. But I think it would also be valid to remove the comdats from both modules, as this change (almost) does.


I'd like to push to get the rename implemented. If we have an internal global with a comdat group, the frontend probably put it there for a good reason (the non-leader is probably metadata) and we shouldn't mess with it.

I hadn't considered that ThinLTO might try to import part of a comdat group, splitting the group. That seems like something we shouldn't do.


https://reviews.llvm.org/D31735