From 76d7df65dafb79acb5abb8a70eb2eef8938c3827 Mon Sep 17 00:00:00 2001 From: PaperChalice Date: Mon, 24 Mar 2025 20:26:43 +0800 Subject: [PATCH 1/6] [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> StrategyMap; +class GCStrategyMap { + using MapT = + MapVector, StringMap>; + 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 &operator[](const std::string &GCName) { + return Strategies[GCName]; + } + + std::pair 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(*F.getParent()) && "This pass need module analysis `collector-metadata`!"); auto &Map = - MAMProxy.getCachedResult(*F.getParent()) - ->StrategyMap; + *MAMProxy.getCachedResult(*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(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 Date: Tue, 25 Mar 2025 12:00:50 +0800 Subject: [PATCH 2/6] 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, StringMap>; + MapVector, StringMap>; MapT Strategies; public: @@ -182,15 +182,13 @@ class GCStrategyMap { bool empty() const { return Strategies.empty(); } - std::unique_ptr &operator[](const std::string &GCName) { - return Strategies[GCName]; - } + GCStrategy &operator[](StringRef GCName) { return *Strategies[GCName]; } - std::pair try_emplace(const std::string &GCName) { + std::pair 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 Date: Tue, 25 Mar 2025 13:41:00 +0800 Subject: [PATCH 3/6] 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(*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 Date: Tue, 25 Mar 2025 13:43:15 +0800 Subject: [PATCH 4/6] 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 parseIR(LLVMContext &Context, const char *IR) { + SMDiagnostic Err; + return parseAssemblyString(IR, Err, Context); +} + +class GCMetadataTest : public ::testing::Test { +protected: + LLVMContext Context; + std::unique_ptr 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(*M); + for (auto &[GCName, Strategy] : StrategyMap) + EXPECT_EQ(GCName, Strategy->getName()); + for (auto &[GCName, Strategy] : llvm::reverse(StrategyMap)) + EXPECT_EQ(GCName, Strategy->getName()); +} + +} // namespace From eebd5328ce206bd15a4e19f99ca1b6d0b4d74f08 Mon Sep 17 00:00:00 2001 From: PaperChalice Date: Wed, 14 May 2025 15:54:23 +0800 Subject: [PATCH 5/6] convert operator[] to const member operator --- llvm/include/llvm/CodeGen/GCMetadata.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/CodeGen/GCMetadata.h b/llvm/include/llvm/CodeGen/GCMetadata.h index a3b284ef0ca30..88e3377dd77b8 100644 --- a/llvm/include/llvm/CodeGen/GCMetadata.h +++ b/llvm/include/llvm/CodeGen/GCMetadata.h @@ -182,7 +182,11 @@ class GCStrategyMap { bool empty() const { return Strategies.empty(); } - GCStrategy &operator[](StringRef GCName) { return *Strategies[GCName]; } + const GCStrategy &operator[](StringRef GCName) const { + auto I = Strategies.find(GCName); + assert(I != Strategies.end() && "Required strategy doesn't exist!"); + return *I->second; + } std::pair try_emplace(StringRef GCName) { return Strategies.try_emplace(GCName); From 916d58e34f135e2a64885a85d42487f3b0356be2 Mon Sep 17 00:00:00 2001 From: PaperChalice Date: Wed, 14 May 2025 16:35:55 +0800 Subject: [PATCH 6/6] Use try_emplace in GCFunctionAnalysis --- llvm/lib/CodeGen/GCMetadata.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/GCMetadata.cpp b/llvm/lib/CodeGen/GCMetadata.cpp index fe3a15b982074..85e07efcdcfc7 100644 --- a/llvm/lib/CodeGen/GCMetadata.cpp +++ b/llvm/lib/CodeGen/GCMetadata.cpp @@ -63,7 +63,8 @@ GCFunctionAnalysis::run(Function &F, FunctionAnalysisManager &FAM) { "This pass need module analysis `collector-metadata`!"); auto &Map = *MAMProxy.getCachedResult(*F.getParent()); - GCFunctionInfo Info(F, Map[F.getGC()]); + GCStrategy &S = *Map.try_emplace(F.getGC()).first->second; + GCFunctionInfo Info(F, S); return Info; }