[llvm] 760bb06 - [ThinLTO] Change GlobalValueSummaryMapTy from std::map to DenseMap+deque (#157839)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 2 19:24:39 PDT 2026
Author: Zhaoxuan Jiang
Date: 2026-07-03T10:24:34+08:00
New Revision: 760bb06cb3cd98832f1e4e7a4eaebd98b66d2bdd
URL: https://github.com/llvm/llvm-project/commit/760bb06cb3cd98832f1e4e7a4eaebd98b66d2bdd
DIFF: https://github.com/llvm/llvm-project/commit/760bb06cb3cd98832f1e4e7a4eaebd98b66d2bdd.diff
LOG: [ThinLTO] Change GlobalValueSummaryMapTy from std::map to DenseMap+deque (#157839)
Replace GlobalValueSummaryMapTy with a custom container using DenseMap
for O(1) lookup and std::deque for storage with pointer stability. Sort
by GUID at serialization points to preserve deterministic output order.
RFC:
https://discourse.llvm.org/t/rfc-change-globalvaluesummarymapty-from-std-map-to-llvm-densemap-for-thin-linking-performance/88191
Added:
Modified:
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 7858bd1f015d9..2b2c14f69c3d5 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -26,6 +26,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/iterator.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/GlobalValue.h"
@@ -41,6 +42,7 @@
#include <cassert>
#include <cstddef>
#include <cstdint>
+#include <deque>
#include <map>
#include <memory>
#include <optional>
@@ -181,13 +183,93 @@ struct alignas(8) GlobalValueSummaryInfo {
};
/// Map from global value GUID to corresponding summary structures. Use a
-/// std::map rather than a DenseMap so that pointers to the map's value_type
-/// (which are used by ValueInfo) are not invalidated by insertion. Also it will
-/// likely incur less overhead, as the value type is not very small and the size
-/// of the map is unknown, resulting in inefficiencies due to repeated
-/// insertions and resizing.
-using GlobalValueSummaryMapTy =
- std::map<GlobalValue::GUID, GlobalValueSummaryInfo>;
+/// DenseMap for O(1) lookup and a std::deque for storage. std::deque
+/// guarantees that pointers to elements are not invalidated by push_back,
+/// which is required because ValueInfo stores a raw pointer to elements of
+/// this container.
+class GlobalValueSummaryMap {
+public:
+ using key_type = GlobalValue::GUID;
+ using mapped_type = GlobalValueSummaryInfo;
+ using value_type = std::pair<key_type, mapped_type>;
+ using iterator = std::deque<value_type>::iterator;
+ using const_iterator = std::deque<value_type>::const_iterator;
+ using size_type = std::deque<value_type>::size_type;
+
+private:
+ /// Vector of pointers into Storage, used for key-sorted iteration.
+ using SortedEntriesVec = SmallVector<const value_type *, 0>;
+
+ DenseMap<key_type, unsigned> Map;
+ std::deque<value_type> Storage;
+
+public:
+ template <typename... Ts>
+ std::pair<iterator, bool> try_emplace(key_type Key, Ts &&...Args) {
+ auto Res = Map.try_emplace(Key, Storage.size());
+ if (Res.second) {
+ Storage.emplace_back(std::piecewise_construct, std::forward_as_tuple(Key),
+ std::forward_as_tuple(std::forward<Ts>(Args)...));
+ return {std::prev(Storage.end()), true};
+ }
+ return {Storage.begin() + Res.first->second, false};
+ }
+
+ iterator find(key_type Key) {
+ auto It = Map.find(Key);
+ return It == Map.end() ? Storage.end() : Storage.begin() + It->second;
+ }
+
+ const_iterator find(key_type Key) const {
+ auto It = Map.find(Key);
+ return It == Map.end() ? Storage.end() : Storage.begin() + It->second;
+ }
+
+ iterator begin() { return Storage.begin(); }
+ const_iterator begin() const { return Storage.begin(); }
+ iterator end() { return Storage.end(); }
+ const_iterator end() const { return Storage.end(); }
+ size_type size() const { return Storage.size(); }
+ bool empty() const { return Storage.empty(); }
+
+ /// An owning range over the entries sorted by key, yielding each entry by
+ /// reference.
+ class SortedEntriesRange {
+ SortedEntriesVec Entries;
+
+ public:
+ using iterator = pointee_iterator<SortedEntriesVec::const_iterator>;
+
+ explicit SortedEntriesRange(SortedEntriesVec Entries)
+ : Entries(std::move(Entries)) {}
+
+ iterator begin() const { return iterator(Entries.begin()); }
+ iterator end() const { return iterator(Entries.end()); }
+ size_t size() const { return Entries.size(); }
+ bool empty() const { return Entries.empty(); }
+ };
+
+ /// Return an owning range over the entries sorted by key. Storage is in
+ /// insertion order; some serialization paths and tests rely on key-sorted
+ /// iteration.
+ SortedEntriesRange sortedRange() const {
+ return SortedEntriesRange(getSortedEntries());
+ }
+
+private:
+ SortedEntriesVec getSortedEntries() const {
+ SortedEntriesVec Sorted;
+ Sorted.reserve(Storage.size());
+ for (const auto &E : Storage)
+ Sorted.push_back(&E);
+ llvm::sort(Sorted, [](const auto *A, const auto *B) {
+ return A->first < B->first;
+ });
+ return Sorted;
+ }
+};
+
+using GlobalValueSummaryMapTy = GlobalValueSummaryMap;
/// Struct that holds a reference to a particular GUID in a global value
/// summary.
@@ -1544,7 +1626,7 @@ class ModuleSummaryIndex {
GlobalValueSummaryMapTy::value_type *
getOrInsertValuePtr(GlobalValue::GUID GUID) {
- return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo(HaveGVs))
+ return &*GlobalValueMap.try_emplace(GUID, GlobalValueSummaryInfo(HaveGVs))
.first;
}
@@ -1583,6 +1665,11 @@ class ModuleSummaryIndex {
const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
size_t size() const { return GlobalValueMap.size(); }
+ GlobalValueSummaryMapTy::SortedEntriesRange
+ sortedGlobalValueSummariesRange() const {
+ return GlobalValueMap.sortedRange();
+ }
+
const std::vector<uint64_t> &stackIds() const { return StackIds; }
unsigned addOrGetStackIdIndex(uint64_t StackId) {
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index 7d50313b948c5..158c03e77306c 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -261,7 +261,8 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
}
}
static void output(IO &io, GlobalValueSummaryMapTy &V) {
- for (auto &P : V) {
+ // Sort by GUID for deterministic output.
+ for (const auto &P : V.sortedRange()) {
std::vector<GlobalValueSummaryYaml> GVSums;
for (auto &Sum : P.second.getSummaryList()) {
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get())) {
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index c4058fe66ff0b..4aa203b0d12d1 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -230,10 +230,12 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
GlobalValueId = VE.getValues().size();
if (!Index)
return;
- for (const auto &GUIDSummaryLists : *Index)
+ // Sort by GUID for deterministic value ID assignment.
+ for (const auto &GUIDSummaryLists :
+ Index->sortedGlobalValueSummariesRange())
// Examine all summaries for this GUID.
for (auto &Summary : GUIDSummaryLists.second.getSummaryList())
- if (auto FS = dyn_cast<FunctionSummary>(Summary.get())) {
+ if (auto *FS = dyn_cast<FunctionSummary>(Summary.get())) {
// For each call in the function summary, see if the call
// is to a GUID (which means it is for an indirect call,
// otherwise we would have a Value for it). If so, synthesize
@@ -583,7 +585,8 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
Callback({AS->getAliaseeGUID(), &AS->getAliasee()}, true);
}
} else {
- for (auto &Summaries : Index)
+ // Sort by GUID for deterministic output.
+ for (const auto &Summaries : Index.sortedGlobalValueSummariesRange())
for (auto &Summary : Summaries.second.getSummaryList())
Callback({Summaries.first, Summary.get()}, false);
}
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 52ed28f71f615..665d7122af2c7 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -1202,7 +1202,8 @@ int SlotTracker::processIndex() {
// Start numbering the GUIDs after the module ids.
GUIDNext = ModulePathNext;
- for (auto &GlobalList : *TheIndex)
+ // Sort by GUID for deterministic slot assignment.
+ for (const auto &GlobalList : TheIndex->sortedGlobalValueSummariesRange())
CreateGUIDSlot(GlobalList.first);
// Start numbering the TypeIdCompatibleVtables after the GUIDs.
@@ -3229,14 +3230,17 @@ void AssemblyWriter::printModuleSummaryIndex() {
// FIXME: Change AliasSummary to hold a ValueInfo instead of summary pointer
// for aliasee (then update BitcodeWriter.cpp and remove get/setAliaseeGUID).
- for (auto &GlobalList : *TheIndex) {
+ // Sort by GUID for deterministic output matching slot assignment order.
+ auto SortedGVS = TheIndex->sortedGlobalValueSummariesRange();
+
+ for (const auto &GlobalList : SortedGVS) {
auto GUID = GlobalList.first;
for (auto &Summary : GlobalList.second.getSummaryList())
SummaryToGUIDMap[Summary.get()] = GUID;
}
// Print the global value summary entries.
- for (auto &GlobalList : *TheIndex) {
+ for (const auto &GlobalList : SortedGVS) {
auto GUID = GlobalList.first;
auto VI = TheIndex->getValueInfo(GlobalList);
printSummaryInfo(Machine.getGUIDSlot(GUID), VI);
diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
index bbcaa530e8e5b..f7fb6af64c8c1 100644
--- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
+++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
@@ -2524,7 +2524,11 @@ IndexCallsiteContextGraph::IndexCallsiteContextGraph(
// by MemProfTopNImportant. Must be a std::map (not DenseMap) because keys
// must be sorted.
std::map<uint64_t, uint32_t> TotalSizeToContextIdTopNCold;
- for (auto &I : Index) {
+ // Sort by GUID for deterministic graph construction order.
+ // TODO: This sort has a measurable cost on the thin link when memprof is
+ // enabled. Investigate gating it behind an option that is only enabled for
+ // tests that check internal state.
+ for (const auto &I : Index.sortedGlobalValueSummariesRange()) {
auto VI = Index.getValueInfo(I);
if (GUIDsToSkip.contains(VI.getGUID()))
continue;
More information about the llvm-commits
mailing list