Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

No description provided.

@kazutakahirata kazutakahirata requested a review from nikic October 16, 2024 03:38
@llvmbot llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Oct 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2024

@llvm/pr-subscribers-lto

Author: Kazu Hirata (kazutakahirata)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/112470.diff

1 Files Affected:

  • (modified) llvm/lib/LTO/LTO.cpp (+3-2)
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 90c4e2c3cd131c..e6846ea9912ca8 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -635,11 +635,12 @@ void LTO::addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
     SymbolResolution Res = *ResI++;
 
     StringRef SymbolName = Sym.getName();
+    auto [It, Inserted] = GlobalResolutions->try_emplace(SymbolName);
     // Keep copies of symbols if the client of LTO says so.
-    if (GlobalResolutionSymbolSaver && !GlobalResolutions->contains(SymbolName))
+    if (GlobalResolutionSymbolSaver && Inserted)
       SymbolName = GlobalResolutionSymbolSaver->save(SymbolName);
 
-    auto &GlobalRes = (*GlobalResolutions)[SymbolName];
+    auto &GlobalRes = It->second;
     GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr();
     if (Res.Prevailing) {
       assert(!GlobalRes.Prevailing &&

SymbolName = GlobalResolutionSymbolSaver->save(SymbolName);

auto &GlobalRes = (*GlobalResolutions)[SymbolName];
auto &GlobalRes = It->second;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this functionally equivalent? Previously if the GlobalResolutions map didn't contain SymbolName then we got a new SymbolName from the symbol saver and created an entry in GlobalResolutions with that. Now we are only indexing GlobalResolutions with the old SymbolName.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this functionally equivalent? Previously if the GlobalResolutions map didn't contain SymbolName then we got a new SymbolName from the symbol saver and created an entry in GlobalResolutions with that. Now we are only indexing GlobalResolutions with the old SymbolName.

Thank you for catching this. You are right. I missed the update to SymbolName.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LTO Link time optimization (regular/full LTO or ThinLTO)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants