change GlobalValueSummaryMapTy from std::map to llvm::MapVector #157839

nocchijiang · 2025-09-10T11:52:50Z

No description provided.

llvmbot · 2025-09-10T11:53:24Z

@llvm/pr-subscribers-llvm-ir

Author: Zhaoxuan Jiang (nocchijiang)

Changes

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

3 Files Affected:

(modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+34-24)
(modified) llvm/include/llvm/IR/ModuleSummaryIndexYAML.h (+4-4)
(modified) llvm/lib/AsmParser/LLParser.cpp (+2-2)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index ac79d91d417c2..d7f8cc1058591 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -17,6 +17,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -177,22 +178,29 @@ struct alignas(8) GlobalValueSummaryInfo {
 /// of the map is unknown, resulting in inefficiencies due to repeated
 /// insertions and resizing.
 using GlobalValueSummaryMapTy =
-    std::map<GlobalValue::GUID, GlobalValueSummaryInfo>;
+    llvm::MapVector<GlobalValue::GUID, GlobalValueSummaryInfo>;
 
 /// Struct that holds a reference to a particular GUID in a global value
 /// summary.
 struct ValueInfo {
   enum Flags { HaveGV = 1, ReadOnly = 2, WriteOnly = 4 };
-  PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 3, int>
+  PointerIntPair<const GlobalValueSummaryMapTy *, 3, int>
       RefAndFlags;
+  PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 1, bool>
+      Offset;
 
   ValueInfo() = default;
-  ValueInfo(bool HaveGVs, const GlobalValueSummaryMapTy::value_type *R) {
-    RefAndFlags.setPointer(R);
+  ValueInfo(bool HaveGVs, const GlobalValueSummaryMapTy::value_type *R, const GlobalValueSummaryMapTy *Map/* = nullptr*/) {
+    RefAndFlags.setPointer(Map);
     RefAndFlags.setInt(HaveGVs);
+    Offset.setPointer(R);
+    if (R != nullptr && Map != nullptr)
+      Offset.setPointer((const GlobalValueSummaryMapTy::value_type *)((uintptr_t)R - (uintptr_t)Map->begin()));
+    if (R != nullptr)
+      Offset.setInt(true);
   }
 
-  explicit operator bool() const { return getRef(); }
+  explicit operator bool() const { return Offset.getInt(); }
 
   GlobalValue::GUID getGUID() const { return getRef()->first; }
   const GlobalValue *getValue() const {
@@ -243,7 +251,9 @@ struct ValueInfo {
   }
 
   const GlobalValueSummaryMapTy::value_type *getRef() const {
-    return RefAndFlags.getPointer();
+    if (RefAndFlags.getPointer())
+      return RefAndFlags.getPointer()->begin() + (intptr_t)Offset.getPointer() / sizeof(GlobalValueSummaryMapTy::value_type);
+    return Offset.getPointer();
   }
 
   /// Returns the most constraining visibility among summaries. The
@@ -286,11 +296,11 @@ inline bool operator<(const ValueInfo &A, const ValueInfo &B) {
 
 template <> struct DenseMapInfo<ValueInfo> {
   static inline ValueInfo getEmptyKey() {
-    return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-8);
+    return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-8, nullptr);
   }
 
   static inline ValueInfo getTombstoneKey() {
-    return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-16);
+    return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-16, nullptr);
   }
 
   static inline bool isSpecialKey(ValueInfo V) {
@@ -301,7 +311,7 @@ template <> struct DenseMapInfo<ValueInfo> {
     // We are not supposed to mix ValueInfo(s) with different HaveGVs flag
     // in a same container.
     assert(isSpecialKey(L) || isSpecialKey(R) || (L.haveGVs() == R.haveGVs()));
-    return L.getRef() == R.getRef();
+    return L.Offset == R.Offset && L.RefAndFlags.getPointer() == R.RefAndFlags.getPointer();
   }
   static unsigned getHashValue(ValueInfo I) { return hash_value(I.getRef()); }
 };
@@ -1397,7 +1407,7 @@ class ModuleSummaryIndex {
 private:
   /// Map from value name to list of summary instances for values of that
   /// name (may be duplicates in the COMDAT case, e.g.).
-  GlobalValueSummaryMapTy GlobalValueMap;
+  std::unique_ptr<GlobalValueSummaryMapTy> GlobalValueMap = std::make_unique<GlobalValueSummaryMapTy>();
 
   /// Holds strings for combined index, mapping to the corresponding module ID.
   ModulePathStringTableTy ModulePathStringTable;
@@ -1501,7 +1511,7 @@ class ModuleSummaryIndex {
 
   GlobalValueSummaryMapTy::value_type *
   getOrInsertValuePtr(GlobalValue::GUID GUID) {
-    return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo(HaveGVs))
+    return &*GlobalValueMap->try_emplace(GUID, GlobalValueSummaryInfo(HaveGVs))
                  .first;
   }
 
@@ -1534,11 +1544,11 @@ class ModuleSummaryIndex {
   void addBlockCount(uint64_t C) { BlockCount += C; }
   void setBlockCount(uint64_t C) { BlockCount = C; }
 
-  gvsummary_iterator begin() { return GlobalValueMap.begin(); }
-  const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }
-  gvsummary_iterator end() { return GlobalValueMap.end(); }
-  const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
-  size_t size() const { return GlobalValueMap.size(); }
+  gvsummary_iterator begin() { return GlobalValueMap->begin(); }
+  const_gvsummary_iterator begin() const { return GlobalValueMap->begin(); }
+  gvsummary_iterator end() { return GlobalValueMap->end(); }
+  const_gvsummary_iterator end() const { return GlobalValueMap->end(); }
+  size_t size() const { return GlobalValueMap->size(); }
 
   const std::vector<uint64_t> &stackIds() const { return StackIds; }
 
@@ -1610,7 +1620,7 @@ class ModuleSummaryIndex {
       if (!S.second.SummaryList.size() ||
           !isa<FunctionSummary>(S.second.SummaryList.front().get()))
         continue;
-      discoverNodes(ValueInfo(HaveGVs, &S), FunctionHasParent);
+      discoverNodes(ValueInfo(HaveGVs, &S, GlobalValueMap.get()), FunctionHasParent);
     }
 
     SmallVector<FunctionSummary::EdgeTy, 0> Edges;
@@ -1677,18 +1687,18 @@ class ModuleSummaryIndex {
   /// Return a ValueInfo for the index value_type (convenient when iterating
   /// index).
   ValueInfo getValueInfo(const GlobalValueSummaryMapTy::value_type &R) const {
-    return ValueInfo(HaveGVs, &R);
+    return ValueInfo(HaveGVs, &R, GlobalValueMap.get());
   }
 
   /// Return a ValueInfo for GUID if it exists, otherwise return ValueInfo().
   ValueInfo getValueInfo(GlobalValue::GUID GUID) const {
-    auto I = GlobalValueMap.find(GUID);
-    return ValueInfo(HaveGVs, I == GlobalValueMap.end() ? nullptr : &*I);
+    auto I = GlobalValueMap->find(GUID);
+    return ValueInfo(HaveGVs, I == GlobalValueMap->end() ? nullptr : &*I, GlobalValueMap.get());
   }
 
   /// Return a ValueInfo for \p GUID.
   ValueInfo getOrInsertValueInfo(GlobalValue::GUID GUID) {
-    return ValueInfo(HaveGVs, getOrInsertValuePtr(GUID));
+    return ValueInfo(HaveGVs, getOrInsertValuePtr(GUID), GlobalValueMap.get());
   }
 
   // Save a string in the Index. Use before passing Name to
@@ -1701,7 +1711,7 @@ class ModuleSummaryIndex {
     assert(!HaveGVs);
     auto VP = getOrInsertValuePtr(GUID);
     VP->second.U.Name = Name;
-    return ValueInfo(HaveGVs, VP);
+    return ValueInfo(HaveGVs, VP, GlobalValueMap.get());
   }
 
   /// Return a ValueInfo for \p GV and mark it as belonging to GV.
@@ -1709,7 +1719,7 @@ class ModuleSummaryIndex {
     assert(HaveGVs);
     auto VP = getOrInsertValuePtr(GV->getGUID());
     VP->second.U.GV = GV;
-    return ValueInfo(HaveGVs, VP);
+    return ValueInfo(HaveGVs, VP, GlobalValueMap.get());
   }
 
   /// Return the GUID for \p OriginalId in the OidGuidMap.
@@ -2038,7 +2048,7 @@ struct GraphTraits<ModuleSummaryIndex *> : public GraphTraits<ValueInfo> {
     G.SummaryList.push_back(std::move(Root));
     static auto P =
         GlobalValueSummaryMapTy::value_type(GlobalValue::GUID(0), std::move(G));
-    return ValueInfo(I->haveGVs(), &P);
+    return ValueInfo(I->haveGVs(), &P, nullptr);
   }
 };
 } // end namespace llvm
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index 531de514822e8..acf7a1a1ef4a5 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -232,7 +232,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
       if (GVSum.Aliasee) {
         auto ASum = std::make_unique<AliasSummary>(GVFlags);
         V.try_emplace(*GVSum.Aliasee, /*IsAnalysis=*/false);
-        ValueInfo AliaseeVI(/*IsAnalysis=*/false, &*V.find(*GVSum.Aliasee));
+        ValueInfo AliaseeVI(/*IsAnalysis=*/false, &*V.find(*GVSum.Aliasee), &V);
         // Note: Aliasee cannot be filled until all summaries are loaded.
         // This is done in fixAliaseeLinks() which is called in
         // MappingTraits<ModuleSummaryIndex>::mapping().
@@ -244,7 +244,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
       Refs.reserve(GVSum.Refs.size());
       for (auto &RefGUID : GVSum.Refs) {
         auto It = V.try_emplace(RefGUID, /*IsAnalysis=*/false).first;
-        Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*It));
+        Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*It, &V));
       }
       Elem.SummaryList.push_back(std::make_unique<FunctionSummary>(
           GVFlags, /*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
@@ -324,10 +324,10 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
 
 template <> struct MappingTraits<ModuleSummaryIndex> {
   static void mapping(IO &io, ModuleSummaryIndex& index) {
-    io.mapOptional("GlobalValueMap", index.GlobalValueMap);
+    io.mapOptional("GlobalValueMap", *index.GlobalValueMap);
     if (!io.outputting())
       CustomMappingTraits<GlobalValueSummaryMapTy>::fixAliaseeLinks(
-          index.GlobalValueMap);
+          *index.GlobalValueMap);
 
     if (io.outputting()) {
       io.mapOptional("TypeIdMap", index.TypeIdMap);
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 1bc2906f63b07..8d6c0951e8181 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9166,7 +9166,7 @@ bool LLParser::parseTypeIdSummary(TypeIdSummary &TIS) {
 }
 
 static ValueInfo EmptyVI =
-    ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-8);
+    ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-8, nullptr);
 
 /// TypeIdCompatibleVtableEntry
 ///   ::= 'typeidCompatibleVTable' ':' '(' 'name' ':' STRINGCONSTANT ','
@@ -10765,7 +10765,7 @@ bool LLParser::parseGVReference(ValueInfo &VI, unsigned &GVId) {
     VI = NumberedValueInfos[GVId];
   } else
     // We will create a forward reference to the stored location.
-    VI = ValueInfo(false, FwdVIRef);
+    VI = ValueInfo(false, FwdVIRef, nullptr);
 
   if (ReadOnly)
     VI.setReadOnly();

github-actions · 2025-09-10T11:55:36Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions h,cpp -- llvm/include/llvm/IR/ModuleSummaryIndex.h llvm/include/llvm/IR/ModuleSummaryIndexYAML.h llvm/lib/AsmParser/LLParser.cpp

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index d7f8cc105..e476a9eed 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -184,18 +184,18 @@ using GlobalValueSummaryMapTy =
 /// summary.
 struct ValueInfo {
   enum Flags { HaveGV = 1, ReadOnly = 2, WriteOnly = 4 };
-  PointerIntPair<const GlobalValueSummaryMapTy *, 3, int>
-      RefAndFlags;
-  PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 1, bool>
-      Offset;
+  PointerIntPair<const GlobalValueSummaryMapTy *, 3, int> RefAndFlags;
+  PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 1, bool> Offset;
 
   ValueInfo() = default;
-  ValueInfo(bool HaveGVs, const GlobalValueSummaryMapTy::value_type *R, const GlobalValueSummaryMapTy *Map/* = nullptr*/) {
+  ValueInfo(bool HaveGVs, const GlobalValueSummaryMapTy::value_type *R,
+            const GlobalValueSummaryMapTy *Map /* = nullptr*/) {
     RefAndFlags.setPointer(Map);
     RefAndFlags.setInt(HaveGVs);
     Offset.setPointer(R);
     if (R != nullptr && Map != nullptr)
-      Offset.setPointer((const GlobalValueSummaryMapTy::value_type *)((uintptr_t)R - (uintptr_t)Map->begin()));
+      Offset.setPointer((const GlobalValueSummaryMapTy::value_type
+                             *)((uintptr_t)R - (uintptr_t)Map->begin()));
     if (R != nullptr)
       Offset.setInt(true);
   }
@@ -252,7 +252,9 @@ struct ValueInfo {
 
   const GlobalValueSummaryMapTy::value_type *getRef() const {
     if (RefAndFlags.getPointer())
-      return RefAndFlags.getPointer()->begin() + (intptr_t)Offset.getPointer() / sizeof(GlobalValueSummaryMapTy::value_type);
+      return RefAndFlags.getPointer()->begin() +
+             (intptr_t)Offset.getPointer() /
+                 sizeof(GlobalValueSummaryMapTy::value_type);
     return Offset.getPointer();
   }
 
@@ -300,7 +302,8 @@ template <> struct DenseMapInfo<ValueInfo> {
   }
 
   static inline ValueInfo getTombstoneKey() {
-    return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-16, nullptr);
+    return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-16,
+                     nullptr);
   }
 
   static inline bool isSpecialKey(ValueInfo V) {
@@ -311,7 +314,8 @@ template <> struct DenseMapInfo<ValueInfo> {
     // We are not supposed to mix ValueInfo(s) with different HaveGVs flag
     // in a same container.
     assert(isSpecialKey(L) || isSpecialKey(R) || (L.haveGVs() == R.haveGVs()));
-    return L.Offset == R.Offset && L.RefAndFlags.getPointer() == R.RefAndFlags.getPointer();
+    return L.Offset == R.Offset &&
+           L.RefAndFlags.getPointer() == R.RefAndFlags.getPointer();
   }
   static unsigned getHashValue(ValueInfo I) { return hash_value(I.getRef()); }
 };
@@ -1407,7 +1411,8 @@ class ModuleSummaryIndex {
 private:
   /// Map from value name to list of summary instances for values of that
   /// name (may be duplicates in the COMDAT case, e.g.).
-  std::unique_ptr<GlobalValueSummaryMapTy> GlobalValueMap = std::make_unique<GlobalValueSummaryMapTy>();
+  std::unique_ptr<GlobalValueSummaryMapTy> GlobalValueMap =
+      std::make_unique<GlobalValueSummaryMapTy>();
 
   /// Holds strings for combined index, mapping to the corresponding module ID.
   ModulePathStringTableTy ModulePathStringTable;
@@ -1620,7 +1625,8 @@ public:
       if (!S.second.SummaryList.size() ||
           !isa<FunctionSummary>(S.second.SummaryList.front().get()))
         continue;
-      discoverNodes(ValueInfo(HaveGVs, &S, GlobalValueMap.get()), FunctionHasParent);
+      discoverNodes(ValueInfo(HaveGVs, &S, GlobalValueMap.get()),
+                    FunctionHasParent);
     }
 
     SmallVector<FunctionSummary::EdgeTy, 0> Edges;
@@ -1693,7 +1699,8 @@ public:
   /// Return a ValueInfo for GUID if it exists, otherwise return ValueInfo().
   ValueInfo getValueInfo(GlobalValue::GUID GUID) const {
     auto I = GlobalValueMap->find(GUID);
-    return ValueInfo(HaveGVs, I == GlobalValueMap->end() ? nullptr : &*I, GlobalValueMap.get());
+    return ValueInfo(HaveGVs, I == GlobalValueMap->end() ? nullptr : &*I,
+                     GlobalValueMap.get());
   }
 
   /// Return a ValueInfo for \p GUID.

change GlobalValueSummaryMapTy from std::map to llvm::MapVector

81392d2

nocchijiang marked this pull request as draft September 10, 2025 11:52

llvmbot added the llvm:ir label Sep 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

change GlobalValueSummaryMapTy from std::map to llvm::MapVector #157839

change GlobalValueSummaryMapTy from std::map to llvm::MapVector #157839

Uh oh!

nocchijiang commented Sep 10, 2025

Uh oh!

llvmbot commented Sep 10, 2025

Uh oh!

github-actions bot commented Sep 10, 2025

Uh oh!

Uh oh!

change GlobalValueSummaryMapTy from std::map to llvm::MapVector #157839

Are you sure you want to change the base?

change GlobalValueSummaryMapTy from std::map to llvm::MapVector #157839

Uh oh!

Conversation

nocchijiang commented Sep 10, 2025

Uh oh!

llvmbot commented Sep 10, 2025

Uh oh!

github-actions bot commented Sep 10, 2025

Uh oh!

Uh oh!