[GVN] MemorySSA for GVN: add optional AllowMemorySSA
#120982
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
antoniofrighetto
requested review from
alinas,
dtcxzyw,
fhahn,
momchil-velikov and
nikic
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Antonio Frighetto (antoniofrighetto) ChangesPreparatory work to migrate from MemoryDependenceAnalysis towards MemorySSA in GVN. Original patch: https://reviews.llvm.org/D118255. Full diff: https://github.com/llvm/llvm-project/pull/120982.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index be6c0ec5edab07..c8be390799836e 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -77,6 +77,7 @@ struct GVNOptions {
std::optional<bool> AllowLoadInLoopPRE;
std::optional<bool> AllowLoadPRESplitBackedge;
std::optional<bool> AllowMemDep;
+ std::optional<bool> AllowMemorySSA;
GVNOptions() = default;
@@ -108,6 +109,12 @@ struct GVNOptions {
AllowMemDep = MemDep;
return *this;
}
+
+ /// Enables or disables use of MemorySSA.
+ GVNOptions &setMemorySSA(bool MemSSA) {
+ AllowMemorySSA = MemSSA;
+ return *this;
+ }
};
/// The core GVN pass object.
@@ -144,6 +151,7 @@ class GVNPass : public PassInfoMixin<GVNPass> {
bool isLoadInLoopPREEnabled() const;
bool isLoadPRESplitBackedgeEnabled() const;
bool isMemDepEnabled() const;
+ bool isMemorySSAEnabled() const;
/// This class holds the mapping between values and value numbers. It is used
/// as an efficient mechanism to determine the expression-wise equivalence of
@@ -383,9 +391,8 @@ class GVNPass : public PassInfoMixin<GVNPass> {
void assignBlockRPONumber(Function &F);
};
-/// Create a legacy GVN pass. This also allows parameterizing whether or not
-/// MemDep is enabled.
-FunctionPass *createGVNPass(bool NoMemDepAnalysis = false);
+/// Create a legacy GVN pass.
+FunctionPass *createGVNPass();
/// A simple and fast domtree-based GVN pass to hoist common expressions
/// from sibling branches.
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index a936f5381137c6..f4744f05233096 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1039,6 +1039,8 @@ Expected<GVNOptions> parseGVNOptions(StringRef Params) {
Result.setLoadPRESplitBackedge(Enable);
} else if (ParamName == "memdep") {
Result.setMemDep(Enable);
+ } else if (ParamName == "memoryssa") {
+ Result.setMemorySSA(Enable);
} else {
return make_error<StringError>(
formatv("invalid GVN pass parameter '{0}' ", ParamName).str(),
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 9f0b09278edcca..c3f6f606876202 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -527,7 +527,7 @@ FUNCTION_PASS_WITH_PARAMS(
"gvn", "GVNPass", [](GVNOptions Opts) { return GVNPass(Opts); },
parseGVNOptions,
"no-pre;pre;no-load-pre;load-pre;no-split-backedge-load-pre;"
- "split-backedge-load-pre;no-memdep;memdep")
+ "split-backedge-load-pre;no-memdep;memdep;no-memoryssa;memoryssa")
FUNCTION_PASS_WITH_PARAMS(
"hardware-loops", "HardwareLoopsPass",
[](HardwareLoopOptions Opts) { return HardwareLoopsPass(Opts); },
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 229fffe92b99c0..56d6bd710f9306 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -113,6 +113,8 @@ static cl::opt<bool>
GVNEnableSplitBackedgeInLoadPRE("enable-split-backedge-in-load-pre",
cl::init(false));
static cl::opt<bool> GVNEnableMemDep("enable-gvn-memdep", cl::init(true));
+static cl::opt<bool> GVNEnableMemorySSA("enable-gvn-memoryssa",
+ cl::init(false));
static cl::opt<uint32_t> MaxNumDeps(
"gvn-max-num-deps", cl::Hidden, cl::init(100),
@@ -820,6 +822,10 @@ bool GVNPass::isMemDepEnabled() const {
return Options.AllowMemDep.value_or(GVNEnableMemDep);
}
+bool GVNPass::isMemorySSAEnabled() const {
+ return Options.AllowMemorySSA.value_or(GVNEnableMemorySSA);
+}
+
PreservedAnalyses GVNPass::run(Function &F, FunctionAnalysisManager &AM) {
// FIXME: The order of evaluation of these 'getResult' calls is very
// significant! Re-ordering these variables will cause GVN when run alone to
@@ -832,7 +838,8 @@ PreservedAnalyses GVNPass::run(Function &F, FunctionAnalysisManager &AM) {
auto *MemDep =
isMemDepEnabled() ? &AM.getResult<MemoryDependenceAnalysis>(F) : nullptr;
auto &LI = AM.getResult<LoopAnalysis>(F);
- auto *MSSA = AM.getCachedResult<MemorySSAAnalysis>(F);
+ auto *MSSA =
+ isMemorySSAEnabled() ? &AM.getResult<MemorySSAAnalysis>(F) : nullptr;
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
bool Changed = runImpl(F, AC, DT, TLI, AA, MemDep, LI, &ORE,
MSSA ? &MSSA->getMSSA() : nullptr);
@@ -861,7 +868,9 @@ void GVNPass::printPipeline(
OS << (*Options.AllowLoadPRESplitBackedge ? "" : "no-")
<< "split-backedge-load-pre;";
if (Options.AllowMemDep != std::nullopt)
- OS << (*Options.AllowMemDep ? "" : "no-") << "memdep";
+ OS << (*Options.AllowMemDep ? "" : "no-") << "memdep;";
+ if (Options.AllowMemorySSA != std::nullopt)
+ OS << (*Options.AllowMemorySSA ? "" : "no-") << "memoryssa";
OS << '>';
}
@@ -3293,8 +3302,11 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
public:
static char ID; // Pass identification, replacement for typeid
- explicit GVNLegacyPass(bool NoMemDepAnalysis = !GVNEnableMemDep)
- : FunctionPass(ID), Impl(GVNOptions().setMemDep(!NoMemDepAnalysis)) {
+ explicit GVNLegacyPass(bool MemDepAnalysis = GVNEnableMemDep,
+ bool MemSSAAnalysis = GVNEnableMemorySSA)
+ : FunctionPass(ID), Impl(GVNOptions()
+ .setMemDep(MemDepAnalysis)
+ .setMemorySSA(MemSSAAnalysis)) {
initializeGVNLegacyPassPass(*PassRegistry::getPassRegistry());
}
@@ -3302,7 +3314,6 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
if (skipFunction(F))
return false;
- auto *MSSAWP = getAnalysisIfAvailable<MemorySSAWrapperPass>();
return Impl.runImpl(
F, getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F),
getAnalysis<DominatorTreeWrapperPass>().getDomTree(),
@@ -3313,7 +3324,9 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
: nullptr,
getAnalysis<LoopInfoWrapperPass>().getLoopInfo(),
&getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE(),
- MSSAWP ? &MSSAWP->getMSSA() : nullptr);
+ Impl.isMemorySSAEnabled()
+ ? &getAnalysis<MemorySSAWrapperPass>().getMSSA()
+ : nullptr);
}
void getAnalysisUsage(AnalysisUsage &AU) const override {
@@ -3329,7 +3342,8 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
AU.addPreserved<TargetLibraryInfoWrapperPass>();
AU.addPreserved<LoopInfoWrapperPass>();
AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
- AU.addPreserved<MemorySSAWrapperPass>();
+ if (Impl.isMemorySSAEnabled())
+ AU.addRequired<MemorySSAWrapperPass>();
}
private:
@@ -3341,6 +3355,7 @@ char GVNLegacyPass::ID = 0;
INITIALIZE_PASS_BEGIN(GVNLegacyPass, "gvn", "Global Value Numbering", false, false)
INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
INITIALIZE_PASS_DEPENDENCY(MemoryDependenceWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(MemorySSAWrapperPass)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
@@ -3349,6 +3364,4 @@ INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitterWrapperPass)
INITIALIZE_PASS_END(GVNLegacyPass, "gvn", "Global Value Numbering", false, false)
// The public interface to this file...
-FunctionPass *llvm::createGVNPass(bool NoMemDepAnalysis) {
- return new GVNLegacyPass(NoMemDepAnalysis);
-}
+FunctionPass *llvm::createGVNPass() { return new GVNLegacyPass(); }
diff --git a/llvm/test/Other/new-pm-print-pipeline.ll b/llvm/test/Other/new-pm-print-pipeline.ll
index 9016473b36ba44..eb3ffe3a098dda 100644
--- a/llvm/test/Other/new-pm-print-pipeline.ll
+++ b/llvm/test/Other/new-pm-print-pipeline.ll
@@ -31,8 +31,8 @@
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(loop-unroll<>,loop-unroll<partial;peeling;runtime;upperbound;profile-peeling;full-unroll-max=5;O1>,loop-unroll<no-partial;no-peeling;no-runtime;no-upperbound;no-profile-peeling;full-unroll-max=7;O1>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-10
; CHECK-10: function(loop-unroll<O2>,loop-unroll<partial;peeling;runtime;upperbound;profile-peeling;full-unroll-max=5;O1>,loop-unroll<no-partial;no-peeling;no-runtime;no-upperbound;no-profile-peeling;full-unroll-max=7;O1>)
-; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(gvn<>,gvn<pre;load-pre;split-backedge-load-pre;memdep>,gvn<no-pre;no-load-pre;no-split-backedge-load-pre;no-memdep>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-11
-; CHECK-11: function(gvn<>,gvn<pre;load-pre;split-backedge-load-pre;memdep>,gvn<no-pre;no-load-pre;no-split-backedge-load-pre;no-memdep>)
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(gvn<>,gvn<pre;load-pre;split-backedge-load-pre;memdep;memoryssa>,gvn<no-pre;no-load-pre;no-split-backedge-load-pre;no-memdep;no-memoryssa>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-11
+; CHECK-11: function(gvn<>,gvn<pre;load-pre;split-backedge-load-pre;memdep;memoryssa>,gvn<no-pre;no-load-pre;no-split-backedge-load-pre;no-memdep;no-memoryssa>)
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(early-cse<>,early-cse<memssa>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-12
; CHECK-12: function(early-cse<>,early-cse<memssa>)
|
alinas
approved these changes
Jan 8, 2025
Contributor
alinas
left a comment
alinas
left a comment
There was a problem hiding this comment.
I have some reservations that the flag flip can be done in the short term.
Approving as I think this is work needs to eventually be done.
Preparatory work to migrate from MemoryDependenceAnalysis towards MemorySSA in GVN. Co-authored-by: Antonio Frighetto <[email protected]>
antoniofrighetto
force-pushed
the
feature/enable-memssa-gvn
branch
from
549b328 to
eb63cd6
Compare
nikic
reviewed
Jan 10, 2025
Contributor
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
Something here isn't right. There is some compile-time impact and codegen changes: https://llvm-compile-time-tracker.com/compare.php?from=66e41a1a20f2190a800669028a0e80bd86e735ce&to=eb63cd62a4a1907dbd58f12660efd8244e7d81e9&stat=instructions:u
nikic
reviewed
Jan 10, 2025
| auto &LI = AM.getResult<LoopAnalysis>(F); | ||
| auto *MSSA = AM.getCachedResult<MemorySSAAnalysis>(F); | ||
| auto *MSSA = | ||
| isMemorySSAEnabled() ? &AM.getResult<MemorySSAAnalysis>(F) : nullptr; |
Contributor
There was a problem hiding this comment.
The change here means that we no longer try to update and preserve an already computed MSSA instance. I've reverted this patch for now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Preparatory work to migrate from MemoryDependenceAnalysis towards MemorySSA in GVN.
Original patch: https://reviews.llvm.org/D118255.
Minor additions in GVNLegacyPass wrt the original version.