This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Optimize bind opcodes with multiple passes
ClosedPublic

Authored by thevinster on Jul 12 2021, 9:00 PM.

Details

Reviewers
int3
gkm
Group Reviewers
Restricted Project
Commits
rGd695d0d6f605: [lld-macho] Optimize bind opcodes with multiple passes
Summary

In D105866, we used an intermediate container to store a list of opcodes. Here,
we use that data structure to help us perform optimization passes that would allow
a more efficient encoding of bind opcodes. Currently, the functionality mirrors the
optimization pass {1,2} done in ld64 for bind opcodes under optimization gate
to prevent slight regressions.

Diff Detail

Event Timeline

thevinster created this revision.Jul 12 2021, 9:00 PM
Herald added a reviewer: gkm. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
thevinster requested review of this revision.Jul 12 2021, 9:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2021, 9:00 PM
thevinster edited the summary of this revision. (Show Details)Jul 12 2021, 9:05 PM
thevinster edited the summary of this revision. (Show Details)Jul 12 2021, 9:08 PM

There is twice a 15 in your code. Are these magic numbers or could they be named constants?

There is twice a 15 in your code. Are these magic numbers or could they be named constants?

15 is the limit for data to be compacted within a single byte; otherwise it needs to be expanded. It turns out there is already a constant BIND_IMMEDIATE_MASK that already defines these values. I plan to separate pass 3 in a separate diff since that might involve some more changes.

thevinster edited the summary of this revision. (Show Details)

Separate out pass 3 and fix up tests

thevinster edited the summary of this revision. (Show Details)Jul 14 2021, 6:08 PM

Implement optimization gating since multiple passes showed regressions, clang-format

thevinster edited the summary of this revision. (Show Details)Jul 14 2021, 10:18 PM
thevinster retitled this revision from [WIP][lld-macho] Optimize bind opcodes with multiple passes to [lld-macho] Optimize bind opcodes with multiple passes.Jul 14 2021, 10:23 PM

Really stupid question: Are you tied to the order of entries in the vector or could changing the order allow further optimizations?

int3 added a comment.Jul 15 2021, 11:59 AM

@tschuett we already sort the bindings (in sortBindings) so the order should be pretty amenable to optimization

lld/MachO/Driver.cpp
1144 ↗(On Diff #358841)

LLD-ELF defaults this to 1; I think we should follow suit (and gate the bind opcode optimization to > 1)

lld/MachO/SyntheticSections.cpp
370–372

nit: LLVM convention is to omit braces for one-liners

lld/test/MachO/lit.local.cfg
19 ↗(On Diff #358841)

The -O flag should be in the bind-opcodes.s test file, instead of being turned on for all tests

Will fix the rest.

lld/MachO/Driver.cpp
1144 ↗(On Diff #358841)

I was considering that, but when I looked at the definitions of how clang (https://clang.llvm.org/docs/CommandGuide/clang.html#cmdoption-o0) uses the -O flag, they define -O0 as no optimizations which seemed fitting since we don't do any of it right now. That said, this does deviate from the LLD-ELF conventions and I'm happy to take either approaches if you feel strongly that it should be 1.

int3 added inline comments.Jul 15 2021, 3:14 PM
lld/MachO/Driver.cpp
1144 ↗(On Diff #358841)

yeah I think we should follow LLD-ELF more closely than we follow clang

thevinster marked 4 inline comments as done.Jul 15 2021, 5:44 PM
thevinster edited the summary of this revision. (Show Details)

Address comments

int3 accepted this revision.Jul 15 2021, 6:53 PM

Thanks!

This revision is now accepted and ready to land.Jul 15 2021, 6:53 PM

Address minor refactor from D105866

This revision was automatically updated to reflect the committed changes.