[llvm] [GC] Use `MapVector` for `GCStrategyMap` (PR #132729)

via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 24 22:47:17 PDT 2025


https://github.com/paperchalice updated https://github.com/llvm/llvm-project/pull/132729

>From 76d7df65dafb79acb5abb8a70eb2eef8938c3827 Mon Sep 17 00:00:00 2001
From: PaperChalice <liujunchang97 at outlook.com>
Date: Mon, 24 Mar 2025 20:26:43 +0800
Subject: [PATCH 1/4] [GC] Use `MapVector` for `GCStrategyMap`

Use `MapVector`, so `GCStrategyMap` can support forward and reverse iterator, which is required in `AsmPrinter`.
---
 llvm/include/llvm/CodeGen/GCMetadata.h     | 37 ++++++++++++++++++++--
 llvm/lib/CodeGen/GCMetadata.cpp            | 12 +++----
 llvm/lib/CodeGen/ShadowStackGCLowering.cpp |  2 +-
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/GCMetadata.h b/llvm/include/llvm/CodeGen/GCMetadata.h
index ca6a511185c7c..2a4e37930cbe1 100644
--- a/llvm/include/llvm/CodeGen/GCMetadata.h
+++ b/llvm/include/llvm/CodeGen/GCMetadata.h
@@ -33,6 +33,7 @@
 #define LLVM_CODEGEN_GCMETADATA_H
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -151,15 +152,47 @@ class GCFunctionInfo {
   size_t live_size(const iterator &p) const { return roots_size(); }
 };
 
-struct GCStrategyMap {
-  StringMap<std::unique_ptr<GCStrategy>> StrategyMap;
+class GCStrategyMap {
+  using MapT =
+      MapVector<std::string, std::unique_ptr<GCStrategy>, StringMap<unsigned>>;
+  MapT Strategies;
 
+public:
   GCStrategyMap() = default;
   GCStrategyMap(GCStrategyMap &&) = default;
 
   /// Handle invalidation explicitly.
   bool invalidate(Module &M, const PreservedAnalyses &PA,
                   ModuleAnalysisManager::Invalidator &Inv);
+
+  using iterator = MapT::iterator;
+  using const_iterator = MapT::const_iterator;
+  using reverse_iterator = MapT::reverse_iterator;
+  using const_reverse_iterator = MapT::const_reverse_iterator;
+
+  iterator begin() { return Strategies.begin(); }
+  const_iterator begin() const { return Strategies.begin(); }
+  iterator end() { return Strategies.end(); }
+  const_iterator end() const { return Strategies.end(); }
+
+  reverse_iterator rbegin() { return Strategies.rbegin(); }
+  const_reverse_iterator rbegin() const { return Strategies.rbegin(); }
+  reverse_iterator rend() { return Strategies.rend(); }
+  const_reverse_iterator rend() const { return Strategies.rend(); }
+
+  bool empty() const { return Strategies.empty(); }
+
+  std::unique_ptr<GCStrategy> &operator[](const std::string &GCName) {
+    return Strategies[GCName];
+  }
+
+  std::pair<iterator, bool> try_emplace(const std::string &GCName) {
+    return Strategies.try_emplace(GCName);
+  }
+
+  bool contains(const std::string &GCName) const {
+    return Strategies.find(GCName) != Strategies.end();
+  }
 };
 
 /// An analysis pass which caches information about the entire Module.
diff --git a/llvm/lib/CodeGen/GCMetadata.cpp b/llvm/lib/CodeGen/GCMetadata.cpp
index fa87b14e708e1..aec67a8d2e936 100644
--- a/llvm/lib/CodeGen/GCMetadata.cpp
+++ b/llvm/lib/CodeGen/GCMetadata.cpp
@@ -26,7 +26,7 @@ bool GCStrategyMap::invalidate(Module &M, const PreservedAnalyses &PA,
   for (const auto &F : M) {
     if (F.isDeclaration() || !F.hasGC())
       continue;
-    if (!StrategyMap.contains(F.getGC()))
+    if (!contains(F.getGC()))
       return true;
   }
   return false;
@@ -36,17 +36,16 @@ AnalysisKey CollectorMetadataAnalysis::Key;
 
 CollectorMetadataAnalysis::Result
 CollectorMetadataAnalysis::run(Module &M, ModuleAnalysisManager &MAM) {
-  Result R;
-  auto &Map = R.StrategyMap;
+  Result StrategyMap;
   for (auto &F : M) {
     if (F.isDeclaration() || !F.hasGC())
       continue;
     auto GCName = F.getGC();
-    auto [It, Inserted] = Map.try_emplace(GCName);
+    auto [It, Inserted] = StrategyMap.try_emplace(GCName);
     if (Inserted)
       It->second = getGCStrategy(GCName);
   }
-  return R;
+  return StrategyMap;
 }
 
 AnalysisKey GCFunctionAnalysis::Key;
@@ -61,8 +60,7 @@ GCFunctionAnalysis::run(Function &F, FunctionAnalysisManager &FAM) {
       MAMProxy.cachedResultExists<CollectorMetadataAnalysis>(*F.getParent()) &&
       "This pass need module analysis `collector-metadata`!");
   auto &Map =
-      MAMProxy.getCachedResult<CollectorMetadataAnalysis>(*F.getParent())
-          ->StrategyMap;
+      *MAMProxy.getCachedResult<CollectorMetadataAnalysis>(*F.getParent());
   GCFunctionInfo Info(F, *Map[F.getGC()]);
   return Info;
 }
diff --git a/llvm/lib/CodeGen/ShadowStackGCLowering.cpp b/llvm/lib/CodeGen/ShadowStackGCLowering.cpp
index 60c8372577a93..1f9beb84ef62d 100644
--- a/llvm/lib/CodeGen/ShadowStackGCLowering.cpp
+++ b/llvm/lib/CodeGen/ShadowStackGCLowering.cpp
@@ -109,7 +109,7 @@ class ShadowStackGCLowering : public FunctionPass {
 PreservedAnalyses ShadowStackGCLoweringPass::run(Module &M,
                                                  ModuleAnalysisManager &MAM) {
   auto &Map = MAM.getResult<CollectorMetadataAnalysis>(M);
-  if (Map.StrategyMap.contains("shadow-stack"))
+  if (!Map.contains("shadow-stack"))
     return PreservedAnalyses::all();
 
   ShadowStackGCLoweringImpl Impl;

>From 6ab0128dddf43e0888c53738cd7db9873315fe33 Mon Sep 17 00:00:00 2001
From: PaperChalice <liujunchang97 at outlook.com>
Date: Tue, 25 Mar 2025 12:00:50 +0800
Subject: [PATCH 2/4] address comments

---
 llvm/include/llvm/CodeGen/GCMetadata.h | 10 ++++------
 llvm/lib/CodeGen/GCMetadata.cpp        |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/GCMetadata.h b/llvm/include/llvm/CodeGen/GCMetadata.h
index 2a4e37930cbe1..a3b284ef0ca30 100644
--- a/llvm/include/llvm/CodeGen/GCMetadata.h
+++ b/llvm/include/llvm/CodeGen/GCMetadata.h
@@ -154,7 +154,7 @@ class GCFunctionInfo {
 
 class GCStrategyMap {
   using MapT =
-      MapVector<std::string, std::unique_ptr<GCStrategy>, StringMap<unsigned>>;
+      MapVector<StringRef, std::unique_ptr<GCStrategy>, StringMap<unsigned>>;
   MapT Strategies;
 
 public:
@@ -182,15 +182,13 @@ class GCStrategyMap {
 
   bool empty() const { return Strategies.empty(); }
 
-  std::unique_ptr<GCStrategy> &operator[](const std::string &GCName) {
-    return Strategies[GCName];
-  }
+  GCStrategy &operator[](StringRef GCName) { return *Strategies[GCName]; }
 
-  std::pair<iterator, bool> try_emplace(const std::string &GCName) {
+  std::pair<iterator, bool> try_emplace(StringRef GCName) {
     return Strategies.try_emplace(GCName);
   }
 
-  bool contains(const std::string &GCName) const {
+  bool contains(StringRef GCName) const {
     return Strategies.find(GCName) != Strategies.end();
   }
 };
diff --git a/llvm/lib/CodeGen/GCMetadata.cpp b/llvm/lib/CodeGen/GCMetadata.cpp
index aec67a8d2e936..09c328cafe406 100644
--- a/llvm/lib/CodeGen/GCMetadata.cpp
+++ b/llvm/lib/CodeGen/GCMetadata.cpp
@@ -40,7 +40,7 @@ CollectorMetadataAnalysis::run(Module &M, ModuleAnalysisManager &MAM) {
   for (auto &F : M) {
     if (F.isDeclaration() || !F.hasGC())
       continue;
-    auto GCName = F.getGC();
+    StringRef GCName = F.getGC();
     auto [It, Inserted] = StrategyMap.try_emplace(GCName);
     if (Inserted)
       It->second = getGCStrategy(GCName);

>From 4e4afb460ea8c7d30ac74b593c4b5b5be3f4a150 Mon Sep 17 00:00:00 2001
From: PaperChalice <liujunchang97 at outlook.com>
Date: Tue, 25 Mar 2025 13:41:00 +0800
Subject: [PATCH 3/4] set GC name

---
 llvm/include/llvm/IR/GCStrategy.h | 1 +
 llvm/lib/CodeGen/GCMetadata.cpp   | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/IR/GCStrategy.h b/llvm/include/llvm/IR/GCStrategy.h
index cbfbe23aaa068..6b813554d6544 100644
--- a/llvm/include/llvm/IR/GCStrategy.h
+++ b/llvm/include/llvm/IR/GCStrategy.h
@@ -63,6 +63,7 @@ class Type;
 class GCStrategy {
 private:
   friend class GCModuleInfo;
+  friend class CollectorMetadataAnalysis;
 
   std::string Name;
 
diff --git a/llvm/lib/CodeGen/GCMetadata.cpp b/llvm/lib/CodeGen/GCMetadata.cpp
index 09c328cafe406..fe3a15b982074 100644
--- a/llvm/lib/CodeGen/GCMetadata.cpp
+++ b/llvm/lib/CodeGen/GCMetadata.cpp
@@ -42,8 +42,10 @@ CollectorMetadataAnalysis::run(Module &M, ModuleAnalysisManager &MAM) {
       continue;
     StringRef GCName = F.getGC();
     auto [It, Inserted] = StrategyMap.try_emplace(GCName);
-    if (Inserted)
+    if (Inserted) {
       It->second = getGCStrategy(GCName);
+      It->second->Name = GCName;
+    }
   }
   return StrategyMap;
 }
@@ -61,7 +63,7 @@ GCFunctionAnalysis::run(Function &F, FunctionAnalysisManager &FAM) {
       "This pass need module analysis `collector-metadata`!");
   auto &Map =
       *MAMProxy.getCachedResult<CollectorMetadataAnalysis>(*F.getParent());
-  GCFunctionInfo Info(F, *Map[F.getGC()]);
+  GCFunctionInfo Info(F, Map[F.getGC()]);
   return Info;
 }
 

>From 3b055dcb00e69d19dfbeac67fef06d1078a27119 Mon Sep 17 00:00:00 2001
From: PaperChalice <liujunchang97 at outlook.com>
Date: Tue, 25 Mar 2025 13:43:15 +0800
Subject: [PATCH 4/4] Add unit test

---
 llvm/unittests/CodeGen/CMakeLists.txt |  1 +
 llvm/unittests/CodeGen/GCMetadata.cpp | 76 +++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 llvm/unittests/CodeGen/GCMetadata.cpp

diff --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index 4f580e7539f4d..cc8ccc1480018 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -29,6 +29,7 @@ add_llvm_unittest(CodeGenTests
   DIETest.cpp
   DroppedVariableStatsMIRTest.cpp
   DwarfStringPoolEntryRefTest.cpp
+  GCMetadata.cpp
   InstrRefLDVTest.cpp
   LowLevelTypeTest.cpp
   LexicalScopesTest.cpp
diff --git a/llvm/unittests/CodeGen/GCMetadata.cpp b/llvm/unittests/CodeGen/GCMetadata.cpp
new file mode 100644
index 0000000000000..a5d8a63d9b555
--- /dev/null
+++ b/llvm/unittests/CodeGen/GCMetadata.cpp
@@ -0,0 +1,76 @@
+//===- llvm/unittest/CodeGen/GCMetadata.cpp -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/GCMetadata.h"
+#include "llvm/Analysis/CGSCCPassManager.h"
+#include "llvm/Analysis/LoopAnalysisManager.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/Passes/PassBuilder.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+std::unique_ptr<Module> parseIR(LLVMContext &Context, const char *IR) {
+  SMDiagnostic Err;
+  return parseAssemblyString(IR, Err, Context);
+}
+
+class GCMetadataTest : public ::testing::Test {
+protected:
+  LLVMContext Context;
+  std::unique_ptr<Module> M;
+
+public:
+  GCMetadataTest()
+      : M(parseIR(Context, R"(
+%Env = type ptr
+
+define void @.main(%Env) gc "shadow-stack" {
+	%Root = alloca %Env
+	call void @llvm.gcroot( ptr %Root, %Env null )
+	unreachable
+}
+
+define void @g() gc "erlang" {
+entry:
+	ret void
+}
+
+declare void @llvm.gcroot(ptr, %Env)
+)")) {}
+};
+
+TEST_F(GCMetadataTest, Basic) {
+  LoopAnalysisManager LAM;
+  FunctionAnalysisManager FAM;
+  CGSCCAnalysisManager CGAM;
+  ModuleAnalysisManager MAM;
+  PassBuilder PB;
+  PB.registerModuleAnalyses(MAM);
+  PB.registerCGSCCAnalyses(CGAM);
+  PB.registerFunctionAnalyses(FAM);
+  PB.registerLoopAnalyses(LAM);
+  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
+
+  ModulePassManager MPM;
+  FunctionPassManager FPM;
+  GCStrategyMap &StrategyMap = MAM.getResult<CollectorMetadataAnalysis>(*M);
+  for (auto &[GCName, Strategy] : StrategyMap)
+    EXPECT_EQ(GCName, Strategy->getName());
+  for (auto &[GCName, Strategy] : llvm::reverse(StrategyMap))
+    EXPECT_EQ(GCName, Strategy->getName());
+}
+
+} // namespace



More information about the llvm-commits mailing list