[Passes] Generalize ShouldRunExtraVectorPasses to allow re-use (NFCI). #118323

fhahn · 2024-12-02T17:04:24Z

Generalize ShouldRunExtraVectorPasses toj ShouldRunExtraPasses, to allow re-use for other transformations. I intend to use this to let LowerMatrixIntrinsicsPass request EarlyCSE if there are any matrix constructs to lower. This should allow enabling LowerMatrixIntrinsics without a noticable compile-time hit.

llvmbot · 2024-12-02T17:05:02Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-ir

Author: Florian Hahn (fhahn)

Changes

Generalize ShouldRunExtraVectorPasses toj ShouldRunExtraPasses, to allow re-use for other transformations. I intend to use this to let LowerMatrixIntrinsicsPass request EarlyCSE if there are any matrix constructs to lower. This should allow enabling LowerMatrixIntrinsics without a noticable compile-time hit.

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

6 Files Affected:

(modified) llvm/include/llvm/IR/PassManager.h (+32)
(modified) llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h (-32)
(modified) llvm/lib/IR/PassManager.cpp (+2)
(modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+2-2)
(modified) llvm/lib/Passes/PassRegistry.def (+2-2)
(modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+2-4)

diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h
index 5dab9d0d0a7979..91730301bbd415 100644
--- a/llvm/include/llvm/IR/PassManager.h
+++ b/llvm/include/llvm/IR/PassManager.h
@@ -935,6 +935,38 @@ struct InvalidateAllAnalysesPass : PassInfoMixin<InvalidateAllAnalysesPass> {
   }
 };
 
+/// A marker analyss to determine if extra passes should be run on demand.
+/// Passes requesting extra transformations to run need to request and preserve
+/// this analysis.
+struct ShouldRunExtraPasses : public AnalysisInfoMixin<ShouldRunExtraPasses> {
+  static AnalysisKey Key;
+  struct Result {
+    bool invalidate(Function &F, const PreservedAnalyses &PA,
+                    FunctionAnalysisManager::Invalidator &) {
+      // Check whether the analysis has been explicitly invalidated. Otherwise,
+      // it remains preserved.
+      auto PAC = PA.getChecker<ShouldRunExtraPasses>();
+      return !PAC.preservedWhenStateless();
+    }
+  };
+
+  Result run(Function &F, FunctionAnalysisManager &FAM) { return Result(); }
+};
+
+/// A pass manager to run a set of extra function passes if the
+/// ShouldRunExtraPasses marker analysis is present. This allows passes to
+/// request additional transformations on demand. An example is extra
+/// simplifications after loop-vectorization, if runtime checks have been added.
+struct ExtraPassManager : public FunctionPassManager {
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
+    auto PA = PreservedAnalyses::all();
+    if (AM.getCachedResult<ShouldRunExtraPasses>(F))
+      PA.intersect(FunctionPassManager::run(F, AM));
+    PA.abandon<ShouldRunExtraPasses>();
+    return PA;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_IR_PASSMANAGER_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h b/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
index 24b6354662955e..9256cf16c0157e 100644
--- a/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
+++ b/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
@@ -80,38 +80,6 @@ class TargetTransformInfo;
 extern cl::opt<bool> EnableLoopInterleaving;
 extern cl::opt<bool> EnableLoopVectorization;
 
-/// A marker to determine if extra passes after loop vectorization should be
-/// run.
-struct ShouldRunExtraVectorPasses
-    : public AnalysisInfoMixin<ShouldRunExtraVectorPasses> {
-  static AnalysisKey Key;
-  struct Result {
-    bool invalidate(Function &F, const PreservedAnalyses &PA,
-                    FunctionAnalysisManager::Invalidator &) {
-      // Check whether the analysis has been explicitly invalidated. Otherwise,
-      // it remains preserved.
-      auto PAC = PA.getChecker<ShouldRunExtraVectorPasses>();
-      return !PAC.preservedWhenStateless();
-    }
-  };
-
-  Result run(Function &F, FunctionAnalysisManager &FAM) { return Result(); }
-};
-
-/// A pass manager to run a set of extra function simplification passes after
-/// vectorization, if requested. LoopVectorize caches the
-/// ShouldRunExtraVectorPasses analysis to request extra simplifications, if
-/// they could be beneficial.
-struct ExtraVectorPassManager : public FunctionPassManager {
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
-    auto PA = PreservedAnalyses::all();
-    if (AM.getCachedResult<ShouldRunExtraVectorPasses>(F))
-      PA.intersect(FunctionPassManager::run(F, AM));
-    PA.abandon<ShouldRunExtraVectorPasses>();
-    return PA;
-  }
-};
-
 struct LoopVectorizeOptions {
   /// If false, consider all loops for interleaving.
   /// If true, only loops that explicitly request interleaving are considered.
diff --git a/llvm/lib/IR/PassManager.cpp b/llvm/lib/IR/PassManager.cpp
index bf8f7906d3368d..4990695108fc70 100644
--- a/llvm/lib/IR/PassManager.cpp
+++ b/llvm/lib/IR/PassManager.cpp
@@ -160,3 +160,5 @@ void llvm::printIRUnitNameForStackTrace<Function>(raw_ostream &OS,
 AnalysisSetKey CFGAnalyses::SetKey;
 
 AnalysisSetKey PreservedAnalyses::AllAnalysesKey;
+
+AnalysisKey ShouldRunExtraPasses::Key;
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 5a7c327de95870..5a8c60adccee34 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1306,8 +1306,8 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
   // Cleanup after the loop optimization passes.
   FPM.addPass(InstCombinePass());
 
+  ExtraPassManager ExtraPasses;
   if (Level.getSpeedupLevel() > 1 && ExtraVectorizerPasses) {
-    ExtraVectorPassManager ExtraPasses;
     // At higher optimization levels, try to clean up any runtime overlap and
     // alignment checks inserted by the vectorizer. We want to track correlated
     // runtime checks for two inner loops in the same outer loop, fold any
@@ -1328,8 +1328,8 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
     ExtraPasses.addPass(
         SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(true)));
     ExtraPasses.addPass(InstCombinePass());
-    FPM.addPass(std::move(ExtraPasses));
   }
+  FPM.addPass(std::move(ExtraPasses));
 
   // Now that we've formed fast to execute loop structures, we do further
   // optimizations. These are run afterward as they might block doing complex
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 7c3798f6462a46..260fbae1d2f87d 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -303,8 +303,8 @@ FUNCTION_ANALYSIS("regions", RegionInfoAnalysis())
 FUNCTION_ANALYSIS("scalar-evolution", ScalarEvolutionAnalysis())
 FUNCTION_ANALYSIS("should-not-run-function-passes",
                   ShouldNotRunFunctionPassesAnalysis())
-FUNCTION_ANALYSIS("should-run-extra-vector-passes",
-                  ShouldRunExtraVectorPasses())
+FUNCTION_ANALYSIS("should-run-extra-passes",
+                  ShouldRunExtraPasses())
 FUNCTION_ANALYSIS("ssp-layout", SSPLayoutAnalysis())
 FUNCTION_ANALYSIS("stack-safety-local", StackSafetyAnalysis())
 FUNCTION_ANALYSIS("target-ir",
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 90312c1a28df3c..2ce293889e88e4 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -442,8 +442,6 @@ using SCEV2ValueTy = DenseMap<const SCEV *, Value *>;
 
 namespace llvm {
 
-AnalysisKey ShouldRunExtraVectorPasses::Key;
-
 /// InnerLoopVectorizer vectorizes loops which contain only one basic
 /// block to a specified vectorization factor (VF).
 /// This class performs the widening of scalars into vectors, or multiple
@@ -10476,8 +10474,8 @@ PreservedAnalyses LoopVectorizePass::run(Function &F,
     // extra simplification passes should be run.
     // TODO: MadeCFGChanges is not a prefect proxy. Extra passes should only
     // be run if runtime checks have been added.
-    AM.getResult<ShouldRunExtraVectorPasses>(F);
-    PA.preserve<ShouldRunExtraVectorPasses>();
+    AM.getResult<ShouldRunExtraPasses>(F);
+    PA.preserve<ShouldRunExtraPasses>();
   } else {
     PA.preserveSet<CFGAnalyses>();
   }

llvmbot · 2024-12-02T17:05:02Z

@llvm/pr-subscribers-vectorizers

Author: Florian Hahn (fhahn)

Changes

Generalize ShouldRunExtraVectorPasses toj ShouldRunExtraPasses, to allow re-use for other transformations. I intend to use this to let LowerMatrixIntrinsicsPass request EarlyCSE if there are any matrix constructs to lower. This should allow enabling LowerMatrixIntrinsics without a noticable compile-time hit.

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

6 Files Affected:

(modified) llvm/include/llvm/IR/PassManager.h (+32)
(modified) llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h (-32)
(modified) llvm/lib/IR/PassManager.cpp (+2)
(modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+2-2)
(modified) llvm/lib/Passes/PassRegistry.def (+2-2)
(modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+2-4)

diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h
index 5dab9d0d0a7979..91730301bbd415 100644
--- a/llvm/include/llvm/IR/PassManager.h
+++ b/llvm/include/llvm/IR/PassManager.h
@@ -935,6 +935,38 @@ struct InvalidateAllAnalysesPass : PassInfoMixin<InvalidateAllAnalysesPass> {
   }
 };
 
+/// A marker analyss to determine if extra passes should be run on demand.
+/// Passes requesting extra transformations to run need to request and preserve
+/// this analysis.
+struct ShouldRunExtraPasses : public AnalysisInfoMixin<ShouldRunExtraPasses> {
+  static AnalysisKey Key;
+  struct Result {
+    bool invalidate(Function &F, const PreservedAnalyses &PA,
+                    FunctionAnalysisManager::Invalidator &) {
+      // Check whether the analysis has been explicitly invalidated. Otherwise,
+      // it remains preserved.
+      auto PAC = PA.getChecker<ShouldRunExtraPasses>();
+      return !PAC.preservedWhenStateless();
+    }
+  };
+
+  Result run(Function &F, FunctionAnalysisManager &FAM) { return Result(); }
+};
+
+/// A pass manager to run a set of extra function passes if the
+/// ShouldRunExtraPasses marker analysis is present. This allows passes to
+/// request additional transformations on demand. An example is extra
+/// simplifications after loop-vectorization, if runtime checks have been added.
+struct ExtraPassManager : public FunctionPassManager {
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
+    auto PA = PreservedAnalyses::all();
+    if (AM.getCachedResult<ShouldRunExtraPasses>(F))
+      PA.intersect(FunctionPassManager::run(F, AM));
+    PA.abandon<ShouldRunExtraPasses>();
+    return PA;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_IR_PASSMANAGER_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h b/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
index 24b6354662955e..9256cf16c0157e 100644
--- a/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
+++ b/llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
@@ -80,38 +80,6 @@ class TargetTransformInfo;
 extern cl::opt<bool> EnableLoopInterleaving;
 extern cl::opt<bool> EnableLoopVectorization;
 
-/// A marker to determine if extra passes after loop vectorization should be
-/// run.
-struct ShouldRunExtraVectorPasses
-    : public AnalysisInfoMixin<ShouldRunExtraVectorPasses> {
-  static AnalysisKey Key;
-  struct Result {
-    bool invalidate(Function &F, const PreservedAnalyses &PA,
-                    FunctionAnalysisManager::Invalidator &) {
-      // Check whether the analysis has been explicitly invalidated. Otherwise,
-      // it remains preserved.
-      auto PAC = PA.getChecker<ShouldRunExtraVectorPasses>();
-      return !PAC.preservedWhenStateless();
-    }
-  };
-
-  Result run(Function &F, FunctionAnalysisManager &FAM) { return Result(); }
-};
-
-/// A pass manager to run a set of extra function simplification passes after
-/// vectorization, if requested. LoopVectorize caches the
-/// ShouldRunExtraVectorPasses analysis to request extra simplifications, if
-/// they could be beneficial.
-struct ExtraVectorPassManager : public FunctionPassManager {
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
-    auto PA = PreservedAnalyses::all();
-    if (AM.getCachedResult<ShouldRunExtraVectorPasses>(F))
-      PA.intersect(FunctionPassManager::run(F, AM));
-    PA.abandon<ShouldRunExtraVectorPasses>();
-    return PA;
-  }
-};
-
 struct LoopVectorizeOptions {
   /// If false, consider all loops for interleaving.
   /// If true, only loops that explicitly request interleaving are considered.
diff --git a/llvm/lib/IR/PassManager.cpp b/llvm/lib/IR/PassManager.cpp
index bf8f7906d3368d..4990695108fc70 100644
--- a/llvm/lib/IR/PassManager.cpp
+++ b/llvm/lib/IR/PassManager.cpp
@@ -160,3 +160,5 @@ void llvm::printIRUnitNameForStackTrace<Function>(raw_ostream &OS,
 AnalysisSetKey CFGAnalyses::SetKey;
 
 AnalysisSetKey PreservedAnalyses::AllAnalysesKey;
+
+AnalysisKey ShouldRunExtraPasses::Key;
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 5a7c327de95870..5a8c60adccee34 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1306,8 +1306,8 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
   // Cleanup after the loop optimization passes.
   FPM.addPass(InstCombinePass());
 
+  ExtraPassManager ExtraPasses;
   if (Level.getSpeedupLevel() > 1 && ExtraVectorizerPasses) {
-    ExtraVectorPassManager ExtraPasses;
     // At higher optimization levels, try to clean up any runtime overlap and
     // alignment checks inserted by the vectorizer. We want to track correlated
     // runtime checks for two inner loops in the same outer loop, fold any
@@ -1328,8 +1328,8 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
     ExtraPasses.addPass(
         SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(true)));
     ExtraPasses.addPass(InstCombinePass());
-    FPM.addPass(std::move(ExtraPasses));
   }
+  FPM.addPass(std::move(ExtraPasses));
 
   // Now that we've formed fast to execute loop structures, we do further
   // optimizations. These are run afterward as they might block doing complex
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 7c3798f6462a46..260fbae1d2f87d 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -303,8 +303,8 @@ FUNCTION_ANALYSIS("regions", RegionInfoAnalysis())
 FUNCTION_ANALYSIS("scalar-evolution", ScalarEvolutionAnalysis())
 FUNCTION_ANALYSIS("should-not-run-function-passes",
                   ShouldNotRunFunctionPassesAnalysis())
-FUNCTION_ANALYSIS("should-run-extra-vector-passes",
-                  ShouldRunExtraVectorPasses())
+FUNCTION_ANALYSIS("should-run-extra-passes",
+                  ShouldRunExtraPasses())
 FUNCTION_ANALYSIS("ssp-layout", SSPLayoutAnalysis())
 FUNCTION_ANALYSIS("stack-safety-local", StackSafetyAnalysis())
 FUNCTION_ANALYSIS("target-ir",
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 90312c1a28df3c..2ce293889e88e4 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -442,8 +442,6 @@ using SCEV2ValueTy = DenseMap<const SCEV *, Value *>;
 
 namespace llvm {
 
-AnalysisKey ShouldRunExtraVectorPasses::Key;
-
 /// InnerLoopVectorizer vectorizes loops which contain only one basic
 /// block to a specified vectorization factor (VF).
 /// This class performs the widening of scalars into vectors, or multiple
@@ -10476,8 +10474,8 @@ PreservedAnalyses LoopVectorizePass::run(Function &F,
     // extra simplification passes should be run.
     // TODO: MadeCFGChanges is not a prefect proxy. Extra passes should only
     // be run if runtime checks have been added.
-    AM.getResult<ShouldRunExtraVectorPasses>(F);
-    PA.preserve<ShouldRunExtraVectorPasses>();
+    AM.getResult<ShouldRunExtraPasses>(F);
+    PA.preserve<ShouldRunExtraPasses>();
   } else {
     PA.preserveSet<CFGAnalyses>();
   }

nikic

I'm concerned about reusing a single, unparameterized marker analysis for multiple extra pass managers.

nikic · 2024-12-02T21:12:05Z

llvm/include/llvm/IR/PassManager.h

This should not be part of PassManager.h, which is widely included.

Moved to a separate file (llvm/Transforms/Utils/ExtraPassManager), but maybe there's a better place or name?

nikic · 2024-12-02T21:12:14Z

llvm/include/llvm/IR/PassManager.h

@@ -935,6 +935,38 @@ struct InvalidateAllAnalysesPass : PassInfoMixin<InvalidateAllAnalysesPass> {
  }
 };

+/// A marker analyss to determine if extra passes should be run on demand.


Suggested change

/// A marker analyss to determine if extra passes should be run on demand.

/// A marker analysis to determine if extra passes should be run on demand.

Fixed, thanks!

nikic · 2024-12-02T21:14:21Z

llvm/lib/Passes/PassBuilderPipelines.cpp

  }
+  FPM.addPass(std::move(ExtraPasses));


Why this move?

This was a consequence of using a single marker, this required the pass manager to run unconditionally to clear the marker set by LV.

github-actions · 2024-12-03T13:11:33Z

✅ With the latest revision this PR passed the C/C++ code formatter.

fhahn · 2024-12-03T13:14:22Z

I'm concerned about reusing a single, unparameterized marker analysis for multiple extra pass managers.

Updated to require each use to create their own marker analysis, not sure if there's a nicer way to avoid some of the extra boilerplate (although it is quite minimal)

nikic · 2024-12-03T17:58:17Z

llvm/include/llvm/Transforms/Utils/ExtraPassManager.h

@@ -0,0 +1,56 @@
+//===- ExtraPassManager.h - Loop pass management -----------------*- C++


Copied header comment :)

Ah yes, missed that. Updated thanks!

nikic · 2024-12-03T18:00:42Z

llvm/include/llvm/Transforms/Utils/ExtraPassManager.h

+/// request additional transformations on demand. An example is extra
+/// simplifications after loop-vectorization, if runtime checks have been added.
+template <typename MarkerTy>
+struct ExtraPassManager : public FunctionPassManager {


Suggested change

struct ExtraPassManager : public FunctionPassManager {

struct ExtraFunctionPassManager : public FunctionPassManager {

I assume the reason that this doesn't cover the extra LoopUnswitch run is that that one needs an LPM instead?

Might still make sense to also move the LPM variant here as well? It can at least use the same ShouldRunExtraPasses.

Yep, update the name and moved ExtraLoopPassManager here as well. Extended ShouldRunExtraPasses to support Loops as well

nikic · 2024-12-03T18:01:17Z

llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h

@@ -201,6 +170,13 @@ void reportVectorizationFailure(const StringRef DebugMsg,
    const StringRef OREMsg, const StringRef ORETag,
    OptimizationRemarkEmitter *ORE, Loop *TheLoop, Instruction *I = nullptr);

+/// A marker analyss to determine if extra passes should be run after loop


Suggested change

/// A marker analyss to determine if extra passes should be run after loop

/// A marker analysis to determine if extra passes should be run after loop

Updated, thanks!

nikic · 2024-12-03T18:02:07Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -134,6 +134,7 @@
 #include "llvm/Support/NativeFormatting.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "llvm/Transforms/Utils/ExtraPassManager.h"


Is this include needed?

Removed, thanks!

nikic

LGTM

nikic · 2024-12-04T14:06:16Z

llvm/include/llvm/Transforms/Scalar/SimpleLoopUnswitch.h

+      public AnalysisInfoMixin<ShouldRunExtraSimpleLoopUnswitch> {
+  static AnalysisKey Key;
+
+  static bool isRequired() { return true; }


Do we need this isRequired? I thought that's only relevant for passes.

nikic · 2024-12-04T14:06:50Z

llvm/include/llvm/Transforms/Utils/ExtraPassManager.h

+//===- ExtraFunctionPassManager.h - Run Optimizations on Demand ---------*- C++
+//-*-===//


Possible to avoid the odd line break here?

Argh, I thought I already fixed this up....

nikic · 2024-12-04T14:07:11Z

llvm/include/llvm/Transforms/Utils/ExtraPassManager.h

+//===- ExtraFunctionPassManager.h - Run Optimizations on Demand ---------*- C++
+//-*-===//
+//
+//


Two empty lines.

Single line now, thanks

nikic · 2024-12-04T14:10:30Z

llvm/include/llvm/Transforms/Utils/ExtraPassManager.h

+    return PA;
+  }
+
+  static bool isRequired() { return true; }


I'd say this should either be on both or neither. I'd expect this to be inherited from FunctionPassManager/LoopPassManager, but maybe my C++-foo is too weak.

Shouldn't be needed, removed ,thanks

Generalize ShouldRunExtraVectorPasses toj ShouldRunExtraPasses, to allow re-use for other transformations. I intend to use this to let LowerMatrixIntrinsicsPass request EarlyCSE if there are any matrix constructs to lower. This should allow enabling LowerMatrixIntrinsics without a noticable compile-time hit.

aeubanks

glad to see this sort of thing, one comment though

aeubanks · 2024-12-09T20:00:19Z

llvm/include/llvm/Transforms/Utils/ExtraPassManager.h

+/// request additional transformations on demand. An example is extra
+/// simplifications after loop-vectorization, if runtime checks have been added.
+template <typename MarkerTy>
+struct ExtraFunctionPassManager : public FunctionPassManager {


the new pass manager data structures very much do not endorse overriding methods via inheritance. I'd prefer ExtraFunctionPassManager to have a FunctionPassManager member instead

I put up #119348, is that what you had in mind?

As suggested post-commit for llvm#118323, adjust the extra pass managers to no inherit from Function/LoopPassManager, but manage the extra passes via member pass managers.

As suggested post-commit for #118323, adjust the extra pass managers to no inherit from Function/LoopPassManager, but manage the extra passes via member pass managers. PR: #119348

fhahn requested review from nikic, alinas, aeubanks, dtcxzyw and paperchalice December 2, 2024 17:04

llvmbot added vectorizers llvm:ir llvm:transforms labels Dec 2, 2024

nikic reviewed Dec 2, 2024

View reviewed changes

fhahn force-pushed the genearlize-extra-vector-passes branch from cb6f7b9 to ab05874 Compare December 3, 2024 13:08

fhahn force-pushed the genearlize-extra-vector-passes branch from ab05874 to b91287d Compare December 3, 2024 13:12

fhahn mentioned this pull request Dec 3, 2024

[Passes] Always run LowerMatrixIntrinsics, request EarlyCSE on demand. #118507

Closed

nikic reviewed Dec 3, 2024

View reviewed changes

fhahn force-pushed the genearlize-extra-vector-passes branch from 9163f47 to c7772d2 Compare December 4, 2024 14:01

nikic approved these changes Dec 4, 2024

View reviewed changes

fhahn added 6 commits December 4, 2024 14:44

!fixup move to separate header

7ca89e8

!fixup fix formatting

26ec459

!fixup move extra loop pass manager and adjust naming & comments

4061b67

!fixup remove include

c58ee07

!fixup clean up headers

ddc5037

fhahn force-pushed the genearlize-extra-vector-passes branch from 55205b2 to ddc5037 Compare December 4, 2024 14:50

fhahn merged commit 9e66206 into llvm:main Dec 4, 2024
6 of 8 checks passed

fhahn deleted the genearlize-extra-vector-passes branch December 4, 2024 16:55

fhahn restored the genearlize-extra-vector-passes branch December 4, 2024 16:55

fhahn deleted the genearlize-extra-vector-passes branch December 4, 2024 16:55

aeubanks reviewed Dec 9, 2024

View reviewed changes

fhahn mentioned this pull request Dec 10, 2024

[Passes] Manage extra passes using inner pass managers (NFC). #119348

Merged

	/// A marker analyss to determine if extra passes should be run on demand.
	/// A marker analysis to determine if extra passes should be run on demand.

		@@ -0,0 +1,56 @@
		//===- ExtraPassManager.h - Loop pass management -----------------*- C++

	struct ExtraPassManager : public FunctionPassManager {
	struct ExtraFunctionPassManager : public FunctionPassManager {

		//===- ExtraFunctionPassManager.h - Run Optimizations on Demand ---------*- C++
		//-*-===//

[Passes] Generalize ShouldRunExtraVectorPasses to allow re-use (NFCI). #118323

[Passes] Generalize ShouldRunExtraVectorPasses to allow re-use (NFCI). #118323

Uh oh!

Conversation

fhahn commented Dec 2, 2024

Uh oh!

llvmbot commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 2, 2024

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhahn commented Dec 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aeubanks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 3, 2024 •

edited

Loading