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

[ben via Phabricator via llvm-commits]

bd1976llvm added a comment.

Trying again... Phabricator didn't like the markup in my last comment attempt.

Hi Aaron,

Sorry. V.busy as warned. Just got around to looking at this again. Great testcase! It also reproduces with LLD.

In D53234#1301765 <https://reviews.llvm.org/D53234#1301765>, @Aaron1011 wrote:

> > so yes the system linker is perfectly legal to remove some parts of a COMDAT group from the final output.
>
> That would seem to defeat the entire point of section groups - their whole purpose is to force certain sections to stay together.


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).

> 
> 
>> Right, but in this case, if the function is removed then the !associated metadata will be dangling and should have no effect on the output object file, no?
> 
> The BFD linker will refuse to link if two combined sections (e.g. with the same name) have different SHF_LINKER_ORDER flags. When the !associated metadata is removed, LLVM wont emit the `SHF_LINK_ORDER` flag for one of the sections.

Thinking out loud a bit here.. consider these solutions to the problem:

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.

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.

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

4. Teach globalDCE to understand the semantics of !associated metadata.
  - Allows the best potential for optimization in LTO, but; this goes against the current design of !associated metadata.

Given the above, I think that 3. would be the path of least resistance for now.


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

https://reviews.llvm.org/D53234