[PATCH] D97585: [InstrProfiling] Use llvm.compiler.used instead of llvm.used for ELF

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D97585#2595131 <https://reviews.llvm.org/D97585#2595131>, @rnk wrote:

> Wait, are you telling me that `__profd/__profc` data can participate in linker GC, and that is not a problem?

Yes on ld.lld, since D96636 <https://reviews.llvm.org/D96636> .  The GC is more useful with D96757 <https://reviews.llvm.org/D96757>, which moved metadata sections which were not within a `comdat any` into a `comdat noduplicates` group.

> I have actually had to optimize the processing of `/INCLUDE:` directives for COFF (D78845 <https://reviews.llvm.org/D78845>), because I assumed that it was critical that `__profd` be treated as a GC root.

Yes. I think getting rid of the local linkage limitation is important for COFF GC.

> But, on ELF, they were never GC roots, and coverage/PGO has worked just fine there.

Yes. For some sections (`__llvm_prf_nm` `__llvm_prf_vnds`), they work just because linkers have such a rule:

> __start_/__stop_ references from a live input section retains all non-SHF_LINK_ORDER non-SHF_GROUP C identifier name sections.

`-z start-stop-gc` can drop the rule, so we need D97649 <https://reviews.llvm.org/D97649> for robustness.

That said, I am still not very confident with GCness for the compressed `__llvm_prf_nm`. I think for PGO it is optional, so whether we retain or discard it probably does not matter much. For coverage mapping it might be a problem.
But this patch and D97649 <https://reviews.llvm.org/D97649> do not change the status quo.

> So, really, all along, llvm.used was only being used to block LLVM IPO transforms like globalopt, not to establish a GC root. And, as you point out, we don't get the GC root behavior for internal `__profd/__profc` globals, and coverage/PGO seems to work fine.



> What about MachO? Should all platforms not treat __profd as a GC root?

Since MachO does not have comdat. I think it has to be conservative and treats every metadata section GC roots (`llvm.used`).

All binary formats with ELF section group compatible semantics ("if section A in a section group is retained, all members in the group are retained as well") should be able to use `llvm.compiler.used`.
I believe COFF can do it, but as you said, `/INCLUDE:` needs fixes to support local linkage `GlobalObject`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97585