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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

> However, Internalizer currently removes non-externally-visible COMDATs when internalizing global objects. The commit which added this behavior https://github.com/llvm-mirror/llvm/commit/4a6d99b0a96d4eb27d89c22e33981ff0344c5737 states that passes like GlobalDCE can legally remove individual COMDAT members.

My reading of that commit (https://reviews.llvm.org/D10679), and looking at the code, is that GlobalDCE would be able to remove these members because they should *all* be removed from the comdat by internalization. The change appears to make it such that internalization cannot proceed if any member of the comdat is externally visible and therefore cannot be internalized. If all members of the comdat can be internalized (i.e. not in the ExternalComdats set), then they can all be internalized and the comdat removed from all members.

Are you seeing a case where after this internalization we are left with an incomplete comdat that still has some members but not others?

> This behavior can end up causing issues involving the BFD linker and MD_Associated metadata, since the removed COMDAT allows GlobalDCE to delete the target of the MD_Associated metadata. Keeping the COMDAT causes the MD_Associated-annotated object and its target to be kept or removed as a unit, which is the correct behavior.

Sorry, I don't follow what is happening - do you have a test case showing the incorrect behavior?


Repository:
  rL LLVM

https://reviews.llvm.org/D53234