This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] LTO: Unset VisibleToRegularObj where possible
ClosedPublic

Authored by int3 on Mar 22 2021, 12:45 PM.

Details

Summary

This allows LLVM's LTO to internalize symbols that are not referenced
directly by regular objects. Naturally, this means we need to track
which symbols are referenced by regular objects. The approach taken here
is similar to LLD-COFF's: like the COFF port, we extend
SymbolTable::insert() to set the isVisibleToRegularObj bit. (LLD-ELF
relies on the Symbol constructor and Symbol::mergeProperties(), but
the Mach-O port does not have a mergeProperties() equivalent.)

From what I can tell, ld64 (which uses libLTO) doesn't do this
optimization at all. I'm not even sure libLTO provides a way to do this.
Not having ld64's behavior as a reference implementation is unfortunate;
instead, I am relying on LLD-ELF/COFF's behavior as references while
erring on the conservative side. In particular, LLD-MachO will only do
this optimization for executables right now.

We also don't attempt it when -flat_namespace is used -- otherwise
we'd need scan the symbol table to find matches for every un-namespaced
symbol reference, which is expensive.

internalize.ll is based off the LLD-ELF tests internalize-basic.ll and
internalize-undef.ll. Looks like @davide added some of LLD-ELF's internalize
tests, so adding him as a reviewer...

Diff Detail

Event Timeline

int3 created this revision.Mar 22 2021, 12:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2021, 12:45 PM
int3 requested review of this revision.Mar 22 2021, 12:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2021, 12:45 PM
int3 added inline comments.Mar 22 2021, 12:47 PM
lld/MachO/LTO.cpp
40–42

this allows us to access the *.internalize.bc files in our test

davide edited reviewers, added: steven_wu; removed: davide.EditedMar 22 2021, 12:50 PM
davide removed a subscriber: davide.

removing davide
+ steven

int3 planned changes to this revision.Mar 22 2021, 4:03 PM
int3 updated this revision to Diff 332781.Mar 23 2021, 1:46 PM
int3 retitled this revision from [lld-macho] LTO: Properly set VisibleToRegularObj to [lld-macho] LTO: Unset VisibleToRegularObj where possible.
int3 edited the summary of this revision. (Show Details)
int3 added a subscriber: davide.

update

int3 updated this revision to Diff 332782.Mar 23 2021, 1:49 PM

remove stray change

gkm accepted this revision.Apr 15 2021, 9:29 AM
gkm added a subscriber: gkm.

LGTM

lld/MachO/Symbols.h
98

Why the bitfield width here? Do we anticipate adding more flags?

This revision is now accepted and ready to land.Apr 15 2021, 9:29 AM
int3 added inline comments.Apr 15 2021, 4:03 PM
lld/MachO/Symbols.h
98

LLD-ELF does have a bunch more flags, I haven't totally vetted them yet to determine if we'll need them. I guess we can skip the bitfield width for now.

This revision was landed with ongoing or failed builds.Apr 15 2021, 6:16 PM
This revision was automatically updated to reflect the committed changes.