[llvm] [NFCI][WPD]Use unique string saver to store type id (PR #106932)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 20 13:53:01 PST 2024


https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/106932

>From 006c9bf7927e57d70f664760cdc9a33624e759df Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Sun, 1 Sep 2024 18:18:35 -0700
Subject: [PATCH 1/7] [NFCI][WPD]Use unique string saver to store type id

---
 llvm/include/llvm/IR/ModuleSummaryIndex.h | 6 +++---
 llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 00934cc1ce6f2d..e14581f1c6ceb4 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1341,7 +1341,7 @@ class ModuleSummaryIndex {
   /// with that type identifier's metadata. Produced by per module summary
   /// analysis and consumed by thin link. For more information, see description
   /// above where TypeIdCompatibleVtableInfo is defined.
-  std::map<std::string, TypeIdCompatibleVtableInfo, std::less<>>
+  std::map<StringRef, TypeIdCompatibleVtableInfo, std::less<>>
       TypeIdCompatibleVtableMap;
 
   /// Mapping from original ID to GUID. If original ID can map to multiple
@@ -1403,7 +1403,7 @@ class ModuleSummaryIndex {
   // 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).
   BumpPtrAllocator Alloc;
-  StringSaver Saver;
+  UniqueStringSaver Saver;
 
   // The total number of basic blocks in the module in the per-module summary or
   // the total number of basic blocks in the LTO unit in the combined index.
@@ -1841,7 +1841,7 @@ class ModuleSummaryIndex {
   /// the ThinLTO backends.
   TypeIdCompatibleVtableInfo &
   getOrInsertTypeIdCompatibleVtableSummary(StringRef TypeId) {
-    return TypeIdCompatibleVtableMap[std::string(TypeId)];
+    return TypeIdCompatibleVtableMap[saveString(TypeId)];
   }
 
   /// For the given \p TypeId, this returns the TypeIdCompatibleVtableMap
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 33ec14b60dd288..bdb4f4403fbf0f 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4150,7 +4150,7 @@ static void writeTypeIdSummaryRecord(SmallVector<uint64_t, 64> &NameVals,
 
 static void writeTypeIdCompatibleVtableSummaryRecord(
     SmallVector<uint64_t, 64> &NameVals, StringTableBuilder &StrtabBuilder,
-    const std::string &Id, const TypeIdCompatibleVtableInfo &Summary,
+    const StringRef Id, const TypeIdCompatibleVtableInfo &Summary,
     ValueEnumerator &VE) {
   NameVals.push_back(StrtabBuilder.add(Id));
   NameVals.push_back(Id.size());

>From 0d5d77145dae50c28eda07f4ef276b6a79bd755f Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 18 Nov 2024 23:40:09 -0800
Subject: [PATCH 2/7] Share string between TypeIdMap and
 TypeIdCompatibleVTableMap

---
 llvm/include/llvm/IR/ModuleSummaryIndex.h     | 16 ++++++++++------
 llvm/include/llvm/IR/ModuleSummaryIndexYAML.h |  2 +-
 llvm/lib/Bitcode/Writer/BitcodeWriter.cpp     |  4 ++--
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index e14581f1c6ceb4..4183f03e185dbf 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1297,7 +1297,7 @@ using GVSummaryPtrSet = std::unordered_set<GlobalValueSummary *>;
 /// Map of a type GUID to type id string and summary (multimap used
 /// in case of GUID conflicts).
 using TypeIdSummaryMapTy =
-    std::multimap<GlobalValue::GUID, std::pair<std::string, TypeIdSummary>>;
+    std::multimap<GlobalValue::GUID, std::pair<StringRef, TypeIdSummary>>;
 
 /// The following data structures summarize type metadata information.
 /// For type metadata overview see https://llvm.org/docs/TypeMetadata.html.
@@ -1403,7 +1403,10 @@ class ModuleSummaryIndex {
   // 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).
   BumpPtrAllocator Alloc;
-  UniqueStringSaver Saver;
+  StringSaver Saver;
+
+  BumpPtrAllocator TypeIdSaverAlloc;
+  UniqueStringSaver TypeIdSaver;
 
   // The total number of basic blocks in the module in the per-module summary or
   // the total number of basic blocks in the LTO unit in the combined index.
@@ -1438,7 +1441,8 @@ class ModuleSummaryIndex {
   ModuleSummaryIndex(bool HaveGVs, bool EnableSplitLTOUnit = false,
                      bool UnifiedLTO = false)
       : HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit),
-        UnifiedLTO(UnifiedLTO), Saver(Alloc), BlockCount(0) {}
+        UnifiedLTO(UnifiedLTO), Saver(Alloc), TypeIdSaver(TypeIdSaverAlloc),
+        BlockCount(0) {}
 
   // Current version for the module summary in bitcode files.
   // The BitcodeSummaryVersion should be bumped whenever we introduce changes
@@ -1811,8 +1815,8 @@ class ModuleSummaryIndex {
     for (auto It = TidIter.first; It != TidIter.second; ++It)
       if (It->second.first == TypeId)
         return It->second.second;
-    auto It = TypeIdMap.insert(
-        {GlobalValue::getGUID(TypeId), {std::string(TypeId), TypeIdSummary()}});
+    auto It = TypeIdMap.insert({GlobalValue::getGUID(TypeId),
+                                {TypeIdSaver.save(TypeId), TypeIdSummary()}});
     return It->second.second;
   }
 
@@ -1841,7 +1845,7 @@ class ModuleSummaryIndex {
   /// the ThinLTO backends.
   TypeIdCompatibleVtableInfo &
   getOrInsertTypeIdCompatibleVtableSummary(StringRef TypeId) {
-    return TypeIdCompatibleVtableMap[saveString(TypeId)];
+    return TypeIdCompatibleVtableMap[TypeIdSaver.save(TypeId)];
   }
 
   /// For the given \p TypeId, this returns the TypeIdCompatibleVtableMap
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index b2747d24c5396d..49c5000e8b469a 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -275,7 +275,7 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
   }
   static void output(IO &io, TypeIdSummaryMapTy &V) {
     for (auto &TidIter : V)
-      io.mapRequired(TidIter.second.first.c_str(), TidIter.second.second);
+      io.mapRequired(TidIter.second.first.str().c_str(), TidIter.second.second);
   }
 };
 
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index bdb4f4403fbf0f..80857058e0c01e 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4131,7 +4131,7 @@ static void writeWholeProgramDevirtResolution(
 
 static void writeTypeIdSummaryRecord(SmallVector<uint64_t, 64> &NameVals,
                                      StringTableBuilder &StrtabBuilder,
-                                     const std::string &Id,
+                                     StringRef Id,
                                      const TypeIdSummary &Summary) {
   NameVals.push_back(StrtabBuilder.add(Id));
   NameVals.push_back(Id.size());
@@ -4150,7 +4150,7 @@ static void writeTypeIdSummaryRecord(SmallVector<uint64_t, 64> &NameVals,
 
 static void writeTypeIdCompatibleVtableSummaryRecord(
     SmallVector<uint64_t, 64> &NameVals, StringTableBuilder &StrtabBuilder,
-    const StringRef Id, const TypeIdCompatibleVtableInfo &Summary,
+    StringRef Id, const TypeIdCompatibleVtableInfo &Summary,
     ValueEnumerator &VE) {
   NameVals.push_back(StrtabBuilder.add(Id));
   NameVals.push_back(Id.size());

>From c8e3dbd9c55033cac30afb03f4b487ec83c1d9d2 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 20 Nov 2024 00:02:16 -0800
Subject: [PATCH 3/7] update index yaml parser

---
 llvm/include/llvm/IR/ModuleSummaryIndexYAML.h | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index 560eca13f6850c..f1d3c437cdfca8 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -313,7 +313,7 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
   static void inputOne(IO &io, StringRef Key, TypeIdSummaryMapTy &V) {
     TypeIdSummary TId;
     io.mapRequired(Key.str().c_str(), TId);
-    V.insert({GlobalValue::getGUID(Key), {std::string(Key), TId}});
+    V.insert({GlobalValue::getGUID(Key), {Key, TId}});
   }
   static void output(IO &io, TypeIdSummaryMapTy &V) {
     for (auto &TidIter : V)
@@ -327,7 +327,20 @@ template <> struct MappingTraits<ModuleSummaryIndex> {
     if (!io.outputting())
       CustomMappingTraits<GlobalValueSummaryMapTy>::fixAliaseeLinks(
           index.GlobalValueMap);
-    io.mapOptional("TypeIdMap", index.TypeIdMap);
+
+    if (io.outputting()) {
+      io.mapOptional("TypeIdMap", index.TypeIdMap);
+    } else {
+      TypeIdSummaryMapTy TypeIdMap;
+      io.mapOptional("TypeIdMap", TypeIdMap);
+      for (auto &[TypeGUID, TypeIdSummaryMap] : TypeIdMap) {
+        // Save type id references in index and point TypeIdMap to use the
+        // references owned by index.
+        StringRef KeyRef = index.TypeIdSaver.save(TypeIdSummaryMap.first);
+        index.TypeIdMap.insert({TypeGUID, {KeyRef, TypeIdSummaryMap.second}});
+      }
+    }
+
     io.mapOptional("WithGlobalValueDeadStripping",
                    index.WithGlobalValueDeadStripping);
 

>From 140cf4ad2667fce1c36f0b4a1d9e4fbe2002a845 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 20 Nov 2024 00:28:29 -0800
Subject: [PATCH 4/7] undo format

---
 llvm/include/llvm/IR/ModuleSummaryIndex.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index bb77fc4512bc2c..208bf8ec4e48ec 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -691,7 +691,7 @@ class AliasSummary : public GlobalValueSummary {
 
   GlobalValueSummary &getAliasee() {
     return const_cast<GlobalValueSummary &>(
-        static_cast<const AliasSummary *>(this)->getAliasee());
+                         static_cast<const AliasSummary *>(this)->getAliasee());
   }
   ValueInfo getAliaseeVI() const {
     assert(AliaseeValueInfo && "Unexpected missing aliasee");
@@ -1579,7 +1579,9 @@ class ModuleSummaryIndex {
   }
 
   bool withAttributePropagation() const { return WithAttributePropagation; }
-  void setWithAttributePropagation() { WithAttributePropagation = true; }
+  void setWithAttributePropagation() {
+    WithAttributePropagation = true;
+  }
 
   bool withDSOLocalPropagation() const { return WithDSOLocalPropagation; }
   void setWithDSOLocalPropagation() { WithDSOLocalPropagation = true; }
@@ -1716,13 +1718,13 @@ class ModuleSummaryIndex {
 
   /// Find the summary for ValueInfo \p VI in module \p ModuleId, or nullptr if
   /// not found.
-  GlobalValueSummary *findSummaryInModule(ValueInfo VI,
-                                          StringRef ModuleId) const {
+  GlobalValueSummary *findSummaryInModule(ValueInfo VI, StringRef ModuleId) const {
     auto SummaryList = VI.getSummaryList();
-    auto Summary = llvm::find_if(
-        SummaryList, [&](const std::unique_ptr<GlobalValueSummary> &Summary) {
-          return Summary->modulePath() == ModuleId;
-        });
+    auto Summary =
+        llvm::find_if(SummaryList,
+                      [&](const std::unique_ptr<GlobalValueSummary> &Summary) {
+                        return Summary->modulePath() == ModuleId;
+                      });
     if (Summary == SummaryList.end())
       return nullptr;
     return Summary->get();

>From 87ab8d0b7417fa2489de1b46232c06bcea6662ac Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 20 Nov 2024 00:34:48 -0800
Subject: [PATCH 5/7] Initialize string savers before its users fwiw

---
 llvm/include/llvm/IR/ModuleSummaryIndex.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 208bf8ec4e48ec..b32aebb25cf5c7 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1351,6 +1351,9 @@ class ModuleSummaryIndex {
   /// Holds strings for combined index, mapping to the corresponding module ID.
   ModulePathStringTableTy ModulePathStringTable;
 
+  BumpPtrAllocator TypeIdSaverAlloc;
+  UniqueStringSaver TypeIdSaver;
+
   /// Mapping from type identifier GUIDs to type identifier and its summary
   /// information. Produced by thin link.
   TypeIdSummaryMapTy TypeIdMap;
@@ -1423,9 +1426,6 @@ class ModuleSummaryIndex {
   BumpPtrAllocator Alloc;
   StringSaver Saver;
 
-  BumpPtrAllocator TypeIdSaverAlloc;
-  UniqueStringSaver TypeIdSaver;
-
   // The total number of basic blocks in the module in the per-module summary or
   // the total number of basic blocks in the LTO unit in the combined index.
   // FIXME: Putting this in the distributed ThinLTO index files breaks LTO
@@ -1458,8 +1458,8 @@ class ModuleSummaryIndex {
   // See HaveGVs variable comment.
   ModuleSummaryIndex(bool HaveGVs, bool EnableSplitLTOUnit = false,
                      bool UnifiedLTO = false)
-      : HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit),
-        UnifiedLTO(UnifiedLTO), Saver(Alloc), TypeIdSaver(TypeIdSaverAlloc) {}
+      : TypeIdSaver(TypeIdSaverAlloc), HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit),
+        UnifiedLTO(UnifiedLTO),  Saver(Alloc) {}
 
   // Current version for the module summary in bitcode files.
   // The BitcodeSummaryVersion should be bumped whenever we introduce changes

>From ebea1faac45df0719b744d2404ad09e0b5e8e581 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 20 Nov 2024 12:08:29 -0800
Subject: [PATCH 6/7] move the map value

---
 llvm/include/llvm/IR/ModuleSummaryIndexYAML.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index f1d3c437cdfca8..b23fd4a72c93b6 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -337,7 +337,8 @@ template <> struct MappingTraits<ModuleSummaryIndex> {
         // Save type id references in index and point TypeIdMap to use the
         // references owned by index.
         StringRef KeyRef = index.TypeIdSaver.save(TypeIdSummaryMap.first);
-        index.TypeIdMap.insert({TypeGUID, {KeyRef, TypeIdSummaryMap.second}});
+        index.TypeIdMap.insert(
+            {TypeGUID, {KeyRef, std::move(TypeIdSummaryMap.second)}});
       }
     }
 

>From a0468601500a4993e540f8d8c901c44df34a3f03 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 20 Nov 2024 13:52:41 -0800
Subject: [PATCH 7/7] fix clang format

---
 llvm/include/llvm/IR/ModuleSummaryIndex.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index b32aebb25cf5c7..a4eb75ceb6930f 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1458,8 +1458,9 @@ class ModuleSummaryIndex {
   // See HaveGVs variable comment.
   ModuleSummaryIndex(bool HaveGVs, bool EnableSplitLTOUnit = false,
                      bool UnifiedLTO = false)
-      : TypeIdSaver(TypeIdSaverAlloc), HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit),
-        UnifiedLTO(UnifiedLTO),  Saver(Alloc) {}
+      : TypeIdSaver(TypeIdSaverAlloc), HaveGVs(HaveGVs),
+        EnableSplitLTOUnit(EnableSplitLTOUnit), UnifiedLTO(UnifiedLTO),
+        Saver(Alloc) {}
 
   // Current version for the module summary in bitcode files.
   // The BitcodeSummaryVersion should be bumped whenever we introduce changes



More information about the llvm-commits mailing list