[PATCH] D53234: Don't remove COMDATs when internalizing global objects

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D53234#1265891, @Aaron1011 wrote:

> > Normally once they are both internalized and removed from the comdat, it shouldn't matter if only one is kept by GlobalDCE. The other is simply an internal function and no longer part of a comdat (so it's not like the comdat is incomplete).
>
> My understanding is that deleting the COMDAT is invalid.


That's not my understanding (at least in general for comdats, this may be a special case, see below), but hopefully Peter will clarify, as he's more familiar with the linker aspects.

> The LLVM Language Reference <https://llvm.org/docs/LangRef.html#comdats> doesn't mention anything about removing COMDATS during LTO.

True, although that section relates more to the semantics of comdat selection for things that stay comdats. I'm not sure the language ref needs to cover allowable optimizations.

> From that perspective, the dangling !associated metadata is simply a consequence of the underlying issue of the COMDAT removal.

Reading through https://llvm.org/docs/LangRef.html#associated-metadata makes me think that comdats involving associated metadata need to be handled a little differently, since the object with the metadata should not be removed unless the object it references is removed, and the language reference specifically says they should be in the same comdat.

So perhaps this code needs to handle comdat containing an object with associated metadata specially (e.g. could just include in the ExternalComdats set)?


https://reviews.llvm.org/D53234