[PATCH] D103043: [Internalize] Rename instead of removal if a to-be-internalized comdat has more than one member

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: Aaron1011, pcc, rnk, tejohnson.
Herald added subscribers: wenlei, hiraditya.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Beside the `comdat any` deduplication feature, instrumentations use comdat to
establish dependencies among a group of sections, to prevent section based
linker garbage collection from discarding some members without discarding all.
LangRef acknowledges this usage with the following wording:

> All global objects that specify this key will only end up in the final object file if the linker chooses that key over some other key.

On ELF, for PGO instrumentation, a `__llvm_prf_cnts` section and its associated
`__llvm_prf_data` section are placed in the same GRP_COMDAT group.  A
`__llvm_prf_data` is usually not referenced and expects the liveness of its
associated `__llvm_prf_cnts` to retain it.

The `setComdat(nullptr)` code (added by D10679 <https://reviews.llvm.org/D10679>) in InternalizePass can break the
use case. The main goal of this patch is to fix the dependency relationship.

I think it makes sense for InternalizePass to internalize a comdat and thus
suppress the deduplication feature, e.g. a relocatable link of a regular LTO can
hide deduplication so that the result object file cannot collide with a
subsequent link with a comdat with the same signature but non-internal.
The approach adopted by this patch tries to keep this semantics working.

On PE-COFF, for a non-external selection symbol, deduplication is naturally
suppressed with link.exe and lld-link. However, this is fuzzy on ELF and I tend
to believe the spec creator has not thought about this use case (see D102973 <https://reviews.llvm.org/D102973>).

GNU ld and gold are still using the "signature is name based" interpretation.
So even if D102973 <https://reviews.llvm.org/D102973> for ld.lld is accepted, for portability, a better approach is
to rename the comdat. A comdat with one single member is the common case,
leaving the comdat can waste (sizeof(Elf64_Shdr)+4*2) bytes, so we optimize by
deleting the comdat; otherwise we rename the comdat.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103043

Files:
  llvm/include/llvm/Transforms/IPO/Internalize.h
  llvm/lib/Transforms/IPO/Internalize.cpp
  llvm/test/Transforms/Internalize/comdat-empty-moduleid.ll
  llvm/test/Transforms/Internalize/comdat.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103043.347486.patch
Type: text/x-patch
Size: 9823 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210524/40c97b6b/attachment.bin>