[PATCH] D105044: [lld-macho] Move ICF earlier to avoid emitting redundant binds

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added a reviewer: gkm.
Herald added a project: lld-macho.
int3 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is a pretty big refactoring diff, so here are the motivations:

Previously, ICF ran after scanRelocations(), where we emitting
bind/rebase opcodes etc. So we had a bunch of redundant leftovers after
ICF. Having ICF run before Writer seems like a better design, and is
what LLD-ELF does, so this diff refactors it accordingly.

However, ICF had two dependencies on things occurring in Writer: 1) it
needs literals to be deduplicated beforehand and 2) it needs to know
which functions have unwind info, which was being handled by
`UnwindInfoSection::prepareRelocations()`.

In order to do literal deduplication earlier, we need to add literal
input sections to their corresponding output sections. So instead of
putting all input sections into the big `inputSections` vector, and then
filtering them by type later on, I've changed things so that literal
sections get added directly to their output sections during the 'gather'
phase. Likewise for compact unwind sections -- they get added directly
to the UnwindInfoSection now. This latter change is not strictly
necessary, but makes it easier for ICF to determine which functions have
unwind info.

One downside of this refactor is that we now make an additional pass
over the unwind info relocations to figure out which functions have
unwind info, since want to know that before `processRelocations()`. I've
made sure to run that extra loop only if ICF is enabled, so there should
be no overhead in non-optimizing runs of the linker.

The upside of all this is that the `inputSections` vector now contains
only ConcatInputSections that are destined for ConcatOutputSections, so
we can clean up a bunch of code that just existed to filter out other
elements from that vector.

I will test for the lack of redundant binds/rebases in the upcoming
cfstring deduplication diff. While binds/rebases can also happen in the
regular `.text` section, they're more common in `.data` sections, so it
seems more natural to test it that way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105044

Files:
  lld/MachO/ConcatOutputSection.cpp
  lld/MachO/ConcatOutputSection.h
  lld/MachO/Driver.cpp
  lld/MachO/ICF.cpp
  lld/MachO/ICF.h
  lld/MachO/InputSection.cpp
  lld/MachO/InputSection.h
  lld/MachO/MarkLive.cpp
  lld/MachO/SyntheticSections.cpp
  lld/MachO/SyntheticSections.h
  lld/MachO/UnwindInfoSection.cpp
  lld/MachO/UnwindInfoSection.h
  lld/MachO/Writer.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105044.354958.patch
Type: text/x-patch
Size: 30215 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210628/30b6ac97/attachment.bin>