Skip to content

Conversation

@vitalybuka
Copy link
Collaborator

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

Created using spr 1.3.4
@llvmbot llvmbot added LTO Link time optimization (regular/full LTO or ThinLTO) llvm:ir labels Mar 7, 2025
@vitalybuka vitalybuka requested review from fmayer and thurstond March 7, 2025 23:58
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2025

@llvm/pr-subscribers-lto

@llvm/pr-subscribers-llvm-ir

Author: Vitaly Buka (vitalybuka)

Changes

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


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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+26)
  • (modified) llvm/lib/LTO/LTO.cpp (+8-12)
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 539c130af33fa..53b8135664e7f 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Module.h"
@@ -1294,6 +1295,25 @@ class CfiFunctionIndex {
 
 public:
   CfiFunctionIndex() = default;
+  class CfiGUIDIterator
+      : public iterator_adaptor_base<
+            CfiGUIDIterator, std::set<std::string, std::less<>>::const_iterator,
+            std::forward_iterator_tag, GlobalValue::GUID> {
+    using base = iterator_adaptor_base<
+        CfiGUIDIterator, std::set<std::string, std::less<>>::const_iterator,
+        std::forward_iterator_tag, GlobalValue::GUID>;
+
+  public:
+    CfiGUIDIterator() = default;
+    explicit CfiGUIDIterator(
+        std::set<std::string, std::less<>>::const_iterator I)
+        : base(std::move(I)) {}
+
+    GlobalValue::GUID operator*() const {
+      return GlobalValue::getGUID(
+          GlobalValue::dropLLVMManglingEscape(*this->wrapped()));
+    }
+  };
 
   template <typename It> CfiFunctionIndex(It B, It E) : Index(B, E) {}
 
@@ -1305,6 +1325,12 @@ class CfiFunctionIndex {
     return Index.end();
   }
 
+  CfiGUIDIterator guid_begin() const { return CfiGUIDIterator(Index.begin()); }
+  CfiGUIDIterator guid_end() const { return CfiGUIDIterator(Index.end()); }
+  iterator_range<CfiGUIDIterator> guids() const {
+    return make_range(guid_begin(), guid_end());
+  }
+
   template <typename... Args> void emplace(Args &&...A) {
     Index.emplace(std::forward<Args>(A)...);
   }
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index dfd4b5188907d..e895a46b8cd77 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1437,12 +1437,10 @@ class InProcessThinBackend : public ThinBackendProc {
                         OnWrite, ShouldEmitImportsFiles, ThinLTOParallelism),
         AddStream(std::move(AddStream)), Cache(std::move(Cache)),
         ShouldEmitIndexFiles(ShouldEmitIndexFiles) {
-    for (auto &Name : CombinedIndex.cfiFunctionDefs())
-      CfiFunctionDefs.insert(
-          GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Name)));
-    for (auto &Name : CombinedIndex.cfiFunctionDecls())
-      CfiFunctionDecls.insert(
-          GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Name)));
+    auto &Defs = CombinedIndex.cfiFunctionDefs();
+    CfiFunctionDefs.insert(Defs.guid_begin(), Defs.guid_end());
+    auto &Decls = CombinedIndex.cfiFunctionDecls();
+    CfiFunctionDecls.insert(Decls.guid_begin(), Decls.guid_end());
   }
 
   virtual Error runThinLTOBackendThread(
@@ -1965,12 +1963,10 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
 
   // Any functions referenced by the jump table in the regular LTO object must
   // be exported.
-  for (auto &Def : ThinLTO.CombinedIndex.cfiFunctionDefs())
-    ExportedGUIDs.insert(
-        GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Def)));
-  for (auto &Decl : ThinLTO.CombinedIndex.cfiFunctionDecls())
-    ExportedGUIDs.insert(
-        GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Decl)));
+  auto &Defs = ThinLTO.CombinedIndex.cfiFunctionDefs();
+  ExportedGUIDs.insert(Defs.guid_begin(), Defs.guid_end());
+  auto &Decls = ThinLTO.CombinedIndex.cfiFunctionDecls();
+  ExportedGUIDs.insert(Decls.guid_begin(), Decls.guid_end());
 
   auto isExported = [&](StringRef ModuleIdentifier, ValueInfo VI) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);

Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 7dd5f23 into main Mar 8, 2025
6 of 11 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfclto-move-guid-calculation-into-cfifunctionindex branch March 8, 2025 02:59
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime-2 running on rocm-worker-hw-02 while building llvm at step 8 "Add check check-llvm".

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

Here is the relevant piece of the build log for the reference
Step 8 (Add check check-llvm) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/136/175' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/unittests/Support/./SupportTests-LLVM-Unit-3486819-136-175.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=175 GTEST_SHARD_INDEX=136 /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/unittests/Support/./SupportTests
--

Script:
--
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/unittests/Support/./SupportTests --gtest_filter=Caching.NoCommit
--
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/llvm/unittests/Support/Caching.cpp:149: Failure
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /tmp/lit-tmp-wokcgfx1/llvm_test_cache/LLVMTest-002beb.tmp.o to /tmp/lit-tmp-wokcgfx1/llvm_test_cache/llvmcache-foo: No such file or directory
)


/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/llvm/unittests/Support/Caching.cpp:149
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /tmp/lit-tmp-wokcgfx1/llvm_test_cache/LLVMTest-002beb.tmp.o to /tmp/lit-tmp-wokcgfx1/llvm_test_cache/llvmcache-foo: No such file or directory
)



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


@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-sie-win running on sie-win-worker while building llvm at step 7 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/6/44' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:Z:\b\llvm-clang-x86_64-sie-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-30008-6-44.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=44 GTEST_SHARD_INDEX=6 Z:\b\llvm-clang-x86_64-sie-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
Z:\b\llvm-clang-x86_64-sie-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=Caching.NoCommit
--
Z:\b\llvm-clang-x86_64-sie-win\llvm-project\llvm\unittests\Support\Caching.cpp(142): error: Value of: AddStream
  Actual: false
Expected: true


Z:\b\llvm-clang-x86_64-sie-win\llvm-project\llvm\unittests\Support\Caching.cpp:142
Value of: AddStream
  Actual: false
Expected: true



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


@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

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

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

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-llvm) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/50/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-29328-50-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=50 C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=Caching.WriteAfterCommit
--
C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\llvm-project\llvm\unittests\Support\Caching.cpp(103): error: Value of: AddStream
  Actual: false
Expected: true


C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\llvm-project\llvm\unittests\Support\Caching.cpp:103
Value of: AddStream
  Actual: false
Expected: true



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


@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 30, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-win-fast running on as-builder-3 while building llvm at step 7 "test-build-unified-tree-check-llvm-unit".

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

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-llvm-unit) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/49/86' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-10284-49-86.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=86 GTEST_SHARD_INDEX=49 C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\unittests\Support\.\SupportTests.exe --gtest_filter=Caching.NoCommit
--
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\unittests\Support\Caching.cpp(142): error: Value of: AddStream
  Actual: false
Expected: true


C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\unittests\Support\Caching.cpp:142
Value of: AddStream
  Actual: false
Expected: true



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


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

Labels

llvm:ir LTO Link time optimization (regular/full LTO or ThinLTO)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants