Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-ir

Author: Kazu Hirata (kazutakahirata)

Changes

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+14-6)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+4-2)
  • (modified) llvm/lib/Transforms/IPO/LowerTypeTests.cpp (+2-3)
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 1cfe7c15f97dbc..2ff5b4a42643d8 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1399,8 +1399,8 @@ class ModuleSummaryIndex {
   /// True if some of the FunctionSummary contains a ParamAccess.
   bool HasParamAccess = false;
 
-  std::set<std::string> CfiFunctionDefs;
-  std::set<std::string> CfiFunctionDecls;
+  std::set<std::string, std::less<>> CfiFunctionDefs;
+  std::set<std::string, std::less<>> CfiFunctionDecls;
 
   // Used in cases where we want to record the name of a global, but
   // don't have the string owned elsewhere (e.g. the Strtab on a module).
@@ -1647,11 +1647,19 @@ class ModuleSummaryIndex {
     return I == OidGuidMap.end() ? 0 : I->second;
   }
 
-  std::set<std::string> &cfiFunctionDefs() { return CfiFunctionDefs; }
-  const std::set<std::string> &cfiFunctionDefs() const { return CfiFunctionDefs; }
+  std::set<std::string, std::less<>> &cfiFunctionDefs() {
+    return CfiFunctionDefs;
+  }
+  const std::set<std::string, std::less<>> &cfiFunctionDefs() const {
+    return CfiFunctionDefs;
+  }
 
-  std::set<std::string> &cfiFunctionDecls() { return CfiFunctionDecls; }
-  const std::set<std::string> &cfiFunctionDecls() const { return CfiFunctionDecls; }
+  std::set<std::string, std::less<>> &cfiFunctionDecls() {
+    return CfiFunctionDecls;
+  }
+  const std::set<std::string, std::less<>> &cfiFunctionDecls() const {
+    return CfiFunctionDecls;
+  }
 
   /// Add a global value summary for a value.
   void addGlobalValueSummary(const GlobalValue &GV,
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 3e82aa7188bd67..91b1917a3c0c99 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7959,7 +7959,8 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
       break;
 
     case bitc::FS_CFI_FUNCTION_DEFS: {
-      std::set<std::string> &CfiFunctionDefs = TheIndex.cfiFunctionDefs();
+      std::set<std::string, std::less<>> &CfiFunctionDefs =
+          TheIndex.cfiFunctionDefs();
       for (unsigned I = 0; I != Record.size(); I += 2)
         CfiFunctionDefs.insert(
             {Strtab.data() + Record[I], static_cast<size_t>(Record[I + 1])});
@@ -7967,7 +7968,8 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
     }
 
     case bitc::FS_CFI_FUNCTION_DECLS: {
-      std::set<std::string> &CfiFunctionDecls = TheIndex.cfiFunctionDecls();
+      std::set<std::string, std::less<>> &CfiFunctionDecls =
+          TheIndex.cfiFunctionDecls();
       for (unsigned I = 0; I != Record.size(); I += 2)
         CfiFunctionDecls.insert(
             {Strtab.data() + Record[I], static_cast<size_t>(Record[I + 1])});
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 70f83892c37390..bfcf491f36ea1f 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -2037,10 +2037,9 @@ bool LowerTypeTestsModule::lower() {
       // have the same name, but it's not the one we are looking for.
       if (F.hasLocalLinkage())
         continue;
-      if (ImportSummary->cfiFunctionDefs().count(std::string(F.getName())))
+      if (ImportSummary->cfiFunctionDefs().count(F.getName()))
         Defs.push_back(&F);
-      else if (ImportSummary->cfiFunctionDecls().count(
-                   std::string(F.getName())))
+      else if (ImportSummary->cfiFunctionDecls().count(F.getName()))
         Decls.push_back(&F);
     }
 

@kazutakahirata kazutakahirata merged commit c784d32 into llvm:main Nov 12, 2024
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_heterogenous_ThinLTO branch November 12, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants