Skip to content

[NFC][IR] Wrap std::set into CfiFunctionIndex #130361

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

Merged

Conversation

vitalybuka
Copy link
Collaborator

Preparation for CFI Index refactoring,
which will fix O(N^2) in ThinLTO indexing.

We need a data structure to lookup by GUID.
Wrapping allow us to change implementation
with minimal changes to users.

Created using spr 1.3.4
@llvmbot llvmbot added the llvm:ir label Mar 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2025

@llvm/pr-subscribers-llvm-ir

Author: Vitaly Buka (vitalybuka)

Changes

Preparation for CFI Index refactoring,
which will fix O(N^2) in ThinLTO indexing.

We need a data structure to lookup by GUID.
Wrapping allow us to change implementation
with minimal changes to users.


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

1 Files Affected:

  • (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+30-14)
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 3c586a1dd21d8..ada6bb259dc37 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1289,6 +1289,30 @@ struct TypeIdSummary {
   std::map<uint64_t, WholeProgramDevirtResolution> WPDRes;
 };
 
+class CfiFunctionIndex {
+  std::set<std::string, std::less<>> Index;
+
+public:
+  CfiFunctionIndex() = default;
+
+  template <typename It>
+  CfiFunctionIndex(It B, It E) : Index(B, E) {}
+
+  std::set<std::string, std::less<>>::const_iterator begin() const {
+    return Index.begin();
+  }
+
+  std::set<std::string, std::less<>>::const_iterator end() const {
+    return Index.end();
+  }
+
+  template <typename... Args> void emplace(Args &&...A) {
+    Index.emplace(std::forward<Args>(A)...);
+  }
+
+  size_t count(StringRef S) const { return Index.count(S); }
+};
+
 /// 160 bits SHA1
 using ModuleHash = std::array<uint32_t, 5>;
 
@@ -1418,8 +1442,8 @@ class ModuleSummaryIndex {
   /// True if some of the FunctionSummary contains a ParamAccess.
   bool HasParamAccess = false;
 
-  std::set<std::string, std::less<>> CfiFunctionDefs;
-  std::set<std::string, std::less<>> CfiFunctionDecls;
+  CfiFunctionIndex CfiFunctionDefs;
+  CfiFunctionIndex 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).
@@ -1667,19 +1691,11 @@ class ModuleSummaryIndex {
     return I == OidGuidMap.end() ? 0 : I->second;
   }
 
-  std::set<std::string, std::less<>> &cfiFunctionDefs() {
-    return CfiFunctionDefs;
-  }
-  const std::set<std::string, std::less<>> &cfiFunctionDefs() const {
-    return CfiFunctionDefs;
-  }
+  CfiFunctionIndex &cfiFunctionDefs() { return CfiFunctionDefs; }
+  const CfiFunctionIndex &cfiFunctionDefs() const { return CfiFunctionDefs; }
 
-  std::set<std::string, std::less<>> &cfiFunctionDecls() {
-    return CfiFunctionDecls;
-  }
-  const std::set<std::string, std::less<>> &cfiFunctionDecls() const {
-    return CfiFunctionDecls;
-  }
+  CfiFunctionIndex &cfiFunctionDecls() { return CfiFunctionDecls; }
+  const CfiFunctionIndex &cfiFunctionDecls() const { return CfiFunctionDecls; }
 
   /// Add a global value summary for a value.
   void addGlobalValueSummary(const GlobalValue &GV,

Copy link

github-actions bot commented Mar 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@vitalybuka vitalybuka requested review from fmayer and thurstond March 7, 2025 22:50
@fmayer
Copy link
Contributor

fmayer commented Mar 7, 2025

Please fix formatting

Created using spr 1.3.4
@vitalybuka
Copy link
Collaborator Author

Please fix formatting

done

@vitalybuka vitalybuka merged commit 0db702a into main Mar 7, 2025
9 of 11 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcir-wrap-stdset-into-cfifunctionindex branch March 7, 2025 23:57
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

LLVM Buildbot has detected a new failure on builder llvm-nvptx64-nvidia-ubuntu running on as-builder-7 while building llvm at step 6 "test-build-unified-tree-check-llvm".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/14246

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-llvm) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/136/175' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/unittests/Support/./SupportTests-LLVM-Unit-3447956-136-175.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=175 GTEST_SHARD_INDEX=136 /home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/unittests/Support/./SupportTests
--

Script:
--
/home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/unittests/Support/./SupportTests --gtest_filter=Caching.NoCommit
--
/home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/llvm-project/llvm/unittests/Support/Caching.cpp:149: Failure
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/lit-tmp-xgavowdx/llvm_test_cache/LLVMTest-78dd91.tmp.o to /home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/lit-tmp-xgavowdx/llvm_test_cache/llvmcache-foo: No such file or directory
)


/home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/llvm-project/llvm/unittests/Support/Caching.cpp:149
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/lit-tmp-xgavowdx/llvm_test_cache/LLVMTest-78dd91.tmp.o to /home/buildbot/worker/as-builder-7/ramdisk/llvm-nvptx64-nvidia-ubuntu/build/lit-tmp-xgavowdx/llvm_test_cache/llvmcache-foo: No such file or directory
)



********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants