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

Tue Nov 27 11:58:16 PST 2018

Aaron1011 added a comment.

> The point of comdat groups is that only one of the groups of sections from each duplicate comdat set is in linked in from the input files. There are no comdat groups in the output of a normal link (excluding -r links).

Even if the COMDAT data itself isn't included in the final object, LLVM should still respect the rules for removing COMDAT objects before it produces the final object. That is, even if the final object does not
show that the objects were once in a COMDAT, LLVM should ensure that the members are always kept or removed as a group.

> 1, Remove the warning from gnu-ld and other linkers.
>  Seems problematic as it seems like there would be ambiguity as to how to order the sections of an output section where SHF_LINK_ORDER is set on some but not all input sections.

I agree. This would also require downstream users (e.g. people building Rust projects) to update their linkers to solve a problem that LLVM could fix on its own.

> 2. Retain the comdats in the output (this patch). Limits the optimization potential of LTO. !associated metadata sections don't need to be in a comdats.

I don't believe that this is the right way to think about this. This patch prevents LLVM from performing an //invalid// optimization - that is, selectively discarding global objects that the IR explicitly indicates should stay together.

> Add GV's with !associated metadata to llvm.compiler.used.
> 
>   Limits the optimization potential of LTO (but not as much as 2.).

LLVM already does this in SanitizerCoverage <https://github.com/llvm-mirror/llvm/blob/e4c346daccc176632a9c10e88a424d126efd0602/lib/Transforms/Instrumentation/SanitizerCoverage.cpp#L588>

Did you mean adding the target of the !associated metadata to  llvm.compiler.used? If so, I don't believe that there's a difference between this option and option 2.

Consider the case of a function that's unused, except for the fact that it's targeted by a global via !associated metadata. If both share a COMDAT, LTO will still be able to remove them from the final object,
since neither is considered used.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53234/new/

https://reviews.llvm.org/D53234