-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lld][macho] Support order cstrings with -order_file #140307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
4d0a735
9d251c4
f39f908
7c08999
b2c4acb
af0f019
2ea09c3
46ddc4a
2bc748e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -246,15 +246,13 @@ DenseMap<const InputSection *, int> CallGraphSort::run() { | |
| } | ||
|
|
||
| std::optional<int> | ||
| macho::PriorityBuilder::getSymbolPriority(const Defined *sym) { | ||
| if (sym->isAbsolute()) | ||
| return std::nullopt; | ||
| macho::PriorityBuilder::getSymbolOrCStringPriority(const StringRef key, | ||
| InputFile *f) { | ||
|
|
||
| auto it = priorities.find(sym->getName()); | ||
| auto it = priorities.find(key); | ||
| if (it == priorities.end()) | ||
| return std::nullopt; | ||
| const SymbolPriorityEntry &entry = it->second; | ||
| const InputFile *f = sym->isec()->getFile(); | ||
| if (!f) | ||
| return entry.anyObjectFile; | ||
| // We don't use toString(InputFile *) here because it returns the full path | ||
|
|
@@ -268,6 +266,13 @@ macho::PriorityBuilder::getSymbolPriority(const Defined *sym) { | |
| return std::min(entry.objectFiles.lookup(filename), entry.anyObjectFile); | ||
| } | ||
|
|
||
| std::optional<int> | ||
| macho::PriorityBuilder::getSymbolPriority(const Defined *sym) { | ||
| if (sym->isAbsolute()) | ||
| return std::nullopt; | ||
| return getSymbolOrCStringPriority(sym->getName(), sym->isec()->getFile()); | ||
| } | ||
|
|
||
| void macho::PriorityBuilder::extractCallGraphProfile() { | ||
| TimeTraceScope timeScope("Extract call graph profile"); | ||
| bool hasOrderFile = !priorities.empty(); | ||
|
|
@@ -300,7 +305,7 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) { | |
| int prio = std::numeric_limits<int>::min(); | ||
| MemoryBufferRef mbref = *buffer; | ||
| for (StringRef line : args::getLines(mbref)) { | ||
| StringRef objectFile, symbol; | ||
| StringRef objectFile, symbolOrCStrHash; | ||
| line = line.take_until([](char c) { return c == '#'; }); // ignore comments | ||
| line = line.ltrim(); | ||
|
|
||
|
|
@@ -315,7 +320,6 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) { | |
|
|
||
| if (cpuType != CPU_TYPE_ANY && cpuType != target->cpuType) | ||
| continue; | ||
|
|
||
| // Drop the CPU type as well as the colon | ||
| if (cpuType != CPU_TYPE_ANY) | ||
| line = line.drop_until([](char c) { return c == ':'; }).drop_front(); | ||
|
|
@@ -330,10 +334,19 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) { | |
| break; | ||
| } | ||
| } | ||
| symbol = line.trim(); | ||
|
|
||
| if (!symbol.empty()) { | ||
| SymbolPriorityEntry &entry = priorities[symbol]; | ||
| // The rest of the line is either <symbol name> or | ||
| // CStringEntryPrefix<cstring hash> | ||
| if (line.starts_with(CStringEntryPrefix)) { | ||
| StringRef possibleHash = line.drop_front(CStringEntryPrefix.size()); | ||
| uint32_t hash = 0; | ||
| if (to_integer(possibleHash, hash)) | ||
| line = possibleHash; | ||
| } | ||
| symbolOrCStrHash = line.trim(); | ||
|
|
||
| if (!symbolOrCStrHash.empty()) { | ||
| SymbolPriorityEntry &entry = priorities[symbolOrCStrHash]; | ||
| if (!objectFile.empty()) | ||
| entry.objectFiles.insert(std::make_pair(objectFile, prio)); | ||
| else | ||
|
|
@@ -388,3 +401,41 @@ macho::PriorityBuilder::buildInputSectionPriorities() { | |
|
|
||
| return sectionPriorities; | ||
| } | ||
|
|
||
| std::vector<StringPiecePair> macho::PriorityBuilder::buildCStringPriorities( | ||
| ArrayRef<CStringInputSection *> inputs) { | ||
| // Split the input strings into hold and cold sets. | ||
| // Order hot set based on -order_file_cstring for performance improvement; | ||
| // TODO: Order cold set of cstrings for compression via BP. | ||
| std::vector<std::pair<int, StringPiecePair>> | ||
| hotStringPrioritiesAndStringPieces; | ||
| std::vector<StringPiecePair> coldStringPieces; | ||
| std::vector<StringPiecePair> orderedStringPieces; | ||
|
|
||
| for (CStringInputSection *isec : inputs) { | ||
| for (const auto &[stringPieceIdx, piece] : llvm::enumerate(isec->pieces)) { | ||
| if (!piece.live) | ||
| continue; | ||
|
|
||
| std::optional<int> priority = getSymbolOrCStringPriority( | ||
| std::to_string(piece.hash), isec->getFile()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now we convert the string hash in the orderfile into an integer (but we don't use the result), then we also convert each hash in the object file to a string to look it up in the priority map. It would be more efficient to create a new priority map,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@ellishg Do you mean just for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we would use For now, I think we can use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /bikeshedding I believe the only character disallowed in the symbolname would be the null character (at least for ELF). More practical constraints on symbol names arise from the programming language used to generate the executable. So in this case I believe most c-like languages that we care about disallow
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Actually we still support Objective-C which commonly has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM to proceed with a different delimiter.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
oh you're right, forgot about the objC symbol names. but on the other hand, all the existing |
||
| if (!priority) | ||
| coldStringPieces.emplace_back(isec, stringPieceIdx); | ||
| else | ||
| hotStringPrioritiesAndStringPieces.emplace_back( | ||
| *priority, std::make_pair(isec, stringPieceIdx)); | ||
| } | ||
| } | ||
|
|
||
| // Order hot set for perf | ||
| llvm::stable_sort(hotStringPrioritiesAndStringPieces); | ||
| for (auto &[priority, stringPiecePair] : hotStringPrioritiesAndStringPieces) | ||
| orderedStringPieces.push_back(stringPiecePair); | ||
|
|
||
| // TODO: Order cold set for compression | ||
|
|
||
| orderedStringPieces.insert(orderedStringPieces.end(), | ||
| coldStringPieces.begin(), coldStringPieces.end()); | ||
|
|
||
| return orderedStringPieces; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good place to report an error if the orderfile cannot be parsed. It might take some refactoring to avoid code duplication