[lld-macho] `-exported_symbols` should hide symbols before LTO runs
ClosedPublic
Actions

Authored by int3 on Jul 23 2022, 10:14 AM.

Details

Reviewers

thevinster

Group Reviewers

Restricted Project

Commits

rG31760e8189c9: [lld-macho] `-exported_symbols` should hide symbols before LTO runs

Summary

We were previously doing it after LTO, which did have the desired effect
of having the un-exported symbols marked as private extern in the final
output binary, but doing it before LTO creates more optimization
opportunities.

One observable difference is that LTO can now elide un-exported symbols
entirely, so they may not even be present as private externs in the
output.

This is also what ld64 implements.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

int3 created this revision.Jul 23 2022, 10:14 AM

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 23 2022, 10:14 AM

Herald added subscribers: ormris, steven_wu, hiraditya, inglorion. · View Herald Transcript

int3 requested review of this revision.Jul 23 2022, 10:14 AM

Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2022, 10:14 AM

Herald added a subscriber: llvm-commits. · View Herald Transcript

int3 added inline comments.Jul 23 2022, 10:17 AM

lld/MachO/Driver.cpp
1645–1648	had to move these up as well as they need to run before `handleExplicitExports`

Harbormaster completed remote builds in B177189: Diff 447084.Jul 23 2022, 10:24 AM

tweak comment

Harbormaster completed remote builds in B177426: Diff 447405.Jul 25 2022, 11:33 AM

int3 planned changes to this revision.Jul 25 2022, 12:24 PM

update

Harbormaster completed remote builds in B177771: Diff 447904.Jul 26 2022, 6:19 PM

thevinster accepted this revision.Jul 28 2022, 11:59 AM

thevinster added a subscriber: thevinster.

thevinster added inline comments.

lld/MachO/Driver.cpp
540	Instead of needing to set `didCompile` on every loop iteration, we could extract `lto->compile` into another variable and set `didCompile` based on the size of that variable? It's minor but feels cleaner this way.