This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Use separate tablegen file for LC_LINKER_OPTION
AbandonedPublic

Authored by keith on Nov 2 2021, 6:22 PM.

Details

Reviewers
gkm
Group Reviewers
Restricted Project
Summary

This cuts another 5 seconds off the top it takes lld to link our large
project. I assume because the parsing of the larger tablegen file
happened many times, when we actually only wanted to handle these 2
options. This is a bit more work to maintain these arguments in 2 places
but they shouldn't change too much to be a big problem.

Diff Detail

Event Timeline

keith created this revision.Nov 2 2021, 6:22 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added subscribers: dang, mgorny. · View Herald Transcript
keith requested review of this revision.Nov 2 2021, 6:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2021, 6:22 PM
keith added inline comments.Nov 2 2021, 6:23 PM
lld/MachO/DriverUtils.cpp
54

I assume there is another, better, way for me to define 2 OptTables in the same file, I would appreciate pointers!

int3 added a subscriber: int3.Nov 4 2021, 11:31 AM
int3 added inline comments.
lld/MachO/DriverUtils.cpp
54

idk if there are more elegant ways of working with OptTables, but I was wondering if it would be simpler to not use an OptTable altogether. Given that there are only two unique prefixes that we need to check for, we could do it by hand -- something like if (s.startswith("-l")) { ... }.

keith added inline comments.Nov 4 2021, 12:28 PM
lld/MachO/DriverUtils.cpp
54

Yea I was considering that too. I do think that would be much simpler. It looks like we're only missing 2 so it shouldn't grow much https://github.com/keith/ld64/blob/2ff40b093f1b20dad35d48480da016bc380707dd/src/ld/Resolver.cpp#L324-L363

keith added inline comments.Nov 4 2021, 7:55 PM
lld/MachO/DriverUtils.cpp
54

Submitted https://reviews.llvm.org/D113235 for discussion

thakis added a subscriber: thakis.Nov 5 2021, 12:46 PM

Oh, interesting. Any opinion on how this compares to the alternate approach taken for this in D78845 for the COFF linker?

keith abandoned this revision.Nov 9 2021, 8:57 AM

Actually landed https://reviews.llvm.org/D113235 instead, lmk what you think there!