[llvm] [BOLT][Linker][NFC] Remove lookupSymbol() in favor of lookupSymbolInfo() (PR #128070)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 20 13:12:04 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-bolt
Author: YongKang Zhu (yozhu)
<details>
<summary>Changes</summary>
Sometimes we need to know the size of a symbol besides its address, so maybe
we can start using the existing `BOLTLinker::lookupSymbolInfo()` (that returns
symbol address and size) and remove `BOLTLinker::lookupSymbol()` (that only
returns symbol address). And for both we need to check return value as it is
wrapped in `std::optional<>`, which makes the difference even smaller.
---
Full diff: https://github.com/llvm/llvm-project/pull/128070.diff
6 Files Affected:
- (modified) bolt/include/bolt/Core/Linker.h (-7)
- (modified) bolt/lib/Core/BinaryFunction.cpp (+10-10)
- (modified) bolt/lib/Rewrite/JITLinkLinker.cpp (+3-3)
- (modified) bolt/lib/Rewrite/RewriteInstance.cpp (+2-2)
- (modified) bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp (+3-2)
- (modified) bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp (+15-7)
``````````diff
diff --git a/bolt/include/bolt/Core/Linker.h b/bolt/include/bolt/Core/Linker.h
index 1e0876a0e13d9..66b3ad18e3c7b 100644
--- a/bolt/include/bolt/Core/Linker.h
+++ b/bolt/include/bolt/Core/Linker.h
@@ -46,13 +46,6 @@ class BOLTLinker {
/// Return the address and size of a symbol or std::nullopt if it cannot be
/// found.
virtual std::optional<SymbolInfo> lookupSymbolInfo(StringRef Name) const = 0;
-
- /// Return the address of a symbol or std::nullopt if it cannot be found.
- std::optional<uint64_t> lookupSymbol(StringRef Name) const {
- if (const auto Info = lookupSymbolInfo(Name))
- return Info->Address;
- return std::nullopt;
- }
};
} // namespace bolt
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 1e427b2df11cf..8385551576098 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -4259,10 +4259,10 @@ void BinaryFunction::updateOutputValues(const BOLTLinker &Linker) {
if (BC.HasRelocations || isInjected()) {
if (hasConstantIsland()) {
- const auto DataAddress =
- Linker.lookupSymbol(getFunctionConstantIslandLabel()->getName());
- assert(DataAddress && "Cannot find function CI symbol");
- setOutputDataAddress(*DataAddress);
+ const auto IslandLabelSymInfo =
+ Linker.lookupSymbolInfo(getFunctionConstantIslandLabel()->getName());
+ assert(IslandLabelSymInfo && "Cannot find function CI symbol");
+ setOutputDataAddress(IslandLabelSymInfo->Address);
for (auto It : Islands->Offsets) {
const uint64_t OldOffset = It.first;
BinaryData *BD = BC.getBinaryDataAtAddress(getAddress() + OldOffset);
@@ -4270,10 +4270,10 @@ void BinaryFunction::updateOutputValues(const BOLTLinker &Linker) {
continue;
MCSymbol *Symbol = It.second;
- const auto NewAddress = Linker.lookupSymbol(Symbol->getName());
- assert(NewAddress && "Cannot find CI symbol");
+ const auto SymInfo = Linker.lookupSymbolInfo(Symbol->getName());
+ assert(SymInfo && "Cannot find CI symbol");
auto &Section = *getCodeSection();
- const auto NewOffset = *NewAddress - Section.getOutputAddress();
+ const auto NewOffset = SymInfo->Address - Section.getOutputAddress();
BD->setOutputLocation(Section, NewOffset);
}
}
@@ -4298,10 +4298,10 @@ void BinaryFunction::updateOutputValues(const BOLTLinker &Linker) {
FF.setAddress(ColdStartSymbolInfo->Address);
FF.setImageSize(ColdStartSymbolInfo->Size);
if (hasConstantIsland()) {
- const auto DataAddress = Linker.lookupSymbol(
+ const auto SymInfo = Linker.lookupSymbolInfo(
getFunctionColdConstantIslandLabel()->getName());
- assert(DataAddress && "Cannot find cold CI symbol");
- setOutputColdDataAddress(*DataAddress);
+ assert(SymInfo && "Cannot find cold CI symbol");
+ setOutputColdDataAddress(SymInfo->Address);
}
}
}
diff --git a/bolt/lib/Rewrite/JITLinkLinker.cpp b/bolt/lib/Rewrite/JITLinkLinker.cpp
index ba483ae4711df..c287dc002623d 100644
--- a/bolt/lib/Rewrite/JITLinkLinker.cpp
+++ b/bolt/lib/Rewrite/JITLinkLinker.cpp
@@ -125,11 +125,11 @@ struct JITLinkLinker::Context : jitlink::JITLinkContext {
std::string SymName = (*Symbol.first).str();
LLVM_DEBUG(dbgs() << "BOLT: looking for " << SymName << "\n");
- if (auto Address = Linker.lookupSymbol(SymName)) {
+ if (auto SymInfo = Linker.lookupSymbolInfo(SymName)) {
LLVM_DEBUG(dbgs() << "Resolved to address 0x"
- << Twine::utohexstr(*Address) << "\n");
+ << Twine::utohexstr(SymInfo->Address) << "\n");
AllResults[Symbol.first] = orc::ExecutorSymbolDef(
- orc::ExecutorAddr(*Address), JITSymbolFlags());
+ orc::ExecutorAddr(SymInfo->Address), JITSymbolFlags());
continue;
}
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 4329235d47049..40f214f840772 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -5907,9 +5907,9 @@ void RewriteInstance::writeEHFrameHeader() {
}
uint64_t RewriteInstance::getNewValueForSymbol(const StringRef Name) {
- auto Value = Linker->lookupSymbol(Name);
+ auto Value = Linker->lookupSymbolInfo(Name);
if (Value)
- return *Value;
+ return Value->Address;
// Return the original value if we haven't emitted the symbol.
BinaryData *BD = BC->getBinaryDataByName(Name);
diff --git a/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp b/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp
index 026f8d35c55c6..059b1239d806b 100644
--- a/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp
+++ b/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp
@@ -68,10 +68,11 @@ void HugifyRuntimeLibrary::link(BinaryContext &BC, StringRef ToolPath,
assert(!RuntimeStartAddress &&
"We don't currently support linking multiple runtime libraries");
- RuntimeStartAddress = Linker.lookupSymbol("__bolt_hugify_self").value_or(0);
- if (!RuntimeStartAddress) {
+ auto StartSymInfo = Linker.lookupSymbolInfo("__bolt_hugify_self");
+ if (!StartSymInfo) {
errs() << "BOLT-ERROR: hugify library does not define __bolt_hugify_self: "
<< LibPath << "\n";
exit(1);
}
+ RuntimeStartAddress = StartSymInfo->Address;
}
diff --git a/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp b/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
index 217b4f23e8572..d6d6ebecd3ec5 100644
--- a/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
+++ b/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
@@ -203,27 +203,35 @@ void InstrumentationRuntimeLibrary::link(
if (BC.isMachO())
return;
- RuntimeFiniAddress = Linker.lookupSymbol("__bolt_instr_fini").value_or(0);
- if (!RuntimeFiniAddress) {
+ std::optional<BOLTLinker::SymbolInfo> FiniSymInfo =
+ Linker.lookupSymbolInfo("__bolt_instr_fini");
+ if (!FiniSymInfo) {
errs() << "BOLT-ERROR: instrumentation library does not define "
"__bolt_instr_fini: "
<< LibPath << "\n";
exit(1);
}
- RuntimeStartAddress = Linker.lookupSymbol("__bolt_instr_start").value_or(0);
- if (!RuntimeStartAddress) {
+ RuntimeFiniAddress = FiniSymInfo->Address;
+
+ std::optional<BOLTLinker::SymbolInfo> StartSymInfo =
+ Linker.lookupSymbolInfo("__bolt_instr_start");
+ if (!StartSymInfo) {
errs() << "BOLT-ERROR: instrumentation library does not define "
"__bolt_instr_start: "
<< LibPath << "\n";
exit(1);
}
+ RuntimeStartAddress = StartSymInfo->Address;
+
outs() << "BOLT-INFO: output linked against instrumentation runtime "
"library, lib entry point is 0x"
<< Twine::utohexstr(RuntimeStartAddress) << "\n";
+
+ std::optional<BOLTLinker::SymbolInfo> ClearSymInfo =
+ Linker.lookupSymbolInfo("__bolt_instr_clear_counters");
+ const uint64_t ClearSymAddress = ClearSymInfo ? ClearSymInfo->Address : 0;
outs() << "BOLT-INFO: clear procedure is 0x"
- << Twine::utohexstr(
- Linker.lookupSymbol("__bolt_instr_clear_counters").value_or(0))
- << "\n";
+ << Twine::utohexstr(ClearSymAddress) << "\n";
emitTablesAsELFNote(BC);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/128070
More information about the llvm-commits
mailing list