[llvm] 4ecc6af - [InstCombine] create a pass options container and add "use-loop-info" argument
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 17 07:30:21 PST 2023
Author: Sanjay Patel
Date: 2023-02-17T10:30:15-05:00
New Revision: 4ecc6af813e2568c3529ea0fe55eaea891d8ab49
URL: https://github.com/llvm/llvm-project/commit/4ecc6af813e2568c3529ea0fe55eaea891d8ab49
DIFF: https://github.com/llvm/llvm-project/commit/4ecc6af813e2568c3529ea0fe55eaea891d8ab49.diff
LOG: [InstCombine] create a pass options container and add "use-loop-info" argument
This is a cleanup/modernization patch requested in D144045 to make loop
analysis a proper optional parameter to the pass rather than a
semi-arbitrary value inherited via the pass pipeline.
It's a bit more complicated than the recent patch I started copying from
(D143980) because InstCombine already has an option for MaxIterations
(added with D71145).
I debated just deleting that option, but it was used by a pair of existing
tests, so I put it into a struct (code largely copied from SimplifyCFG's
implementation) to make the code more flexible for future options
enhancements.
I didn't alter the pass manager invocations of InstCombine in this patch
because the patch was already getting big, but that will be a small
follow-up as noted with the TODO comment.
Differential Revision: https://reviews.llvm.org/D144199
Added:
Modified:
llvm/include/llvm/Transforms/InstCombine/InstCombine.h
llvm/lib/Passes/PassBuilder.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Other/new-pm-print-pipeline.ll
llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll
llvm/test/Transforms/InstCombine/statepoint-cleanup.ll
llvm/test/Transforms/InstCombine/statepoint-iter.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombine.h b/llvm/include/llvm/Transforms/InstCombine/InstCombine.h
index 35a3a8c3218bf..9c6bcc661d933 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombine.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombine.h
@@ -25,13 +25,35 @@
namespace llvm {
+static constexpr unsigned InstCombineDefaultMaxIterations = 1000;
+
+struct InstCombineOptions {
+ bool UseLoopInfo;
+ unsigned MaxIterations;
+
+ InstCombineOptions()
+ : UseLoopInfo(false), MaxIterations(InstCombineDefaultMaxIterations) {}
+
+ InstCombineOptions &setUseLoopInfo(bool Value) {
+ UseLoopInfo = Value;
+ return *this;
+ }
+
+ InstCombineOptions &setMaxIterations(unsigned Value) {
+ MaxIterations = Value;
+ return *this;
+ }
+};
+
class InstCombinePass : public PassInfoMixin<InstCombinePass> {
+private:
InstructionWorklist Worklist;
- const unsigned MaxIterations;
+ InstCombineOptions Options;
public:
- explicit InstCombinePass();
- explicit InstCombinePass(unsigned MaxIterations);
+ explicit InstCombinePass(InstCombineOptions Opts = {});
+ void printPipeline(raw_ostream &OS,
+ function_ref<StringRef(StringRef)> MapClassName2PassName);
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
};
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index dcd002d0be00c..e335e51f73b08 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -778,6 +778,33 @@ Expected<SimplifyCFGOptions> parseSimplifyCFGOptions(StringRef Params) {
return Result;
}
+Expected<InstCombineOptions> parseInstCombineOptions(StringRef Params) {
+ InstCombineOptions Result;
+ while (!Params.empty()) {
+ StringRef ParamName;
+ std::tie(ParamName, Params) = Params.split(';');
+
+ bool Enable = !ParamName.consume_front("no-");
+ if (ParamName == "use-loop-info") {
+ Result.setUseLoopInfo(Enable);
+ } else if (Enable && ParamName.consume_front("max-iterations=")) {
+ APInt MaxIterations;
+ if (ParamName.getAsInteger(0, MaxIterations))
+ return make_error<StringError>(
+ formatv("invalid argument to InstCombine pass max-iterations "
+ "parameter: '{0}' ",
+ ParamName).str(),
+ inconvertibleErrorCode());
+ Result.setMaxIterations((unsigned)MaxIterations.getZExtValue());
+ } else {
+ return make_error<StringError>(
+ formatv("invalid InstCombine pass parameter '{0}' ", ParamName).str(),
+ inconvertibleErrorCode());
+ }
+ }
+ return Result;
+}
+
/// Parser of parameters for LoopVectorize pass.
Expected<LoopVectorizeOptions> parseLoopVectorizeOptions(StringRef Params) {
LoopVectorizeOptions Opts;
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 04126916a84b0..63b398398575f 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -477,6 +477,15 @@ FUNCTION_PASS_WITH_PARAMS("loop-vectorize",
parseLoopVectorizeOptions,
"no-interleave-forced-only;interleave-forced-only;"
"no-vectorize-forced-only;vectorize-forced-only")
+FUNCTION_PASS_WITH_PARAMS("instcombine",
+ "InstCombinePass",
+ [](InstCombineOptions Opts) {
+ return InstCombinePass(Opts);
+ },
+ parseInstCombineOptions,
+ "no-use-loop-info;use-loop-info;"
+ "max-iterations=N"
+ )
FUNCTION_PASS_WITH_PARAMS("mldst-motion",
"MergedLoadStoreMotionPass",
[](MergedLoadStoreMotionOptions Opts) {
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 54392bb6b562f..4f8ea32f171cc 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -129,7 +129,6 @@ DEBUG_COUNTER(VisitCounter, "instcombine-visit",
"Controls which instructions are visited");
// FIXME: these limits eventually should be as low as 2.
-static constexpr unsigned InstCombineDefaultMaxIterations = 1000;
#ifndef NDEBUG
static constexpr unsigned InstCombineDefaultInfiniteLoopThreshold = 100;
#else
@@ -144,11 +143,6 @@ static cl::opt<unsigned> MaxSinkNumUsers(
"instcombine-max-sink-users", cl::init(32),
cl::desc("Maximum number of undroppable users for instruction sinking"));
-static cl::opt<unsigned> LimitMaxIterations(
- "instcombine-max-iterations",
- cl::desc("Limit the maximum number of instruction combining iterations"),
- cl::init(InstCombineDefaultMaxIterations));
-
static cl::opt<unsigned> InfiniteLoopDetectionThreshold(
"instcombine-infinite-loop-threshold",
cl::desc("Number of instruction combining iterations considered an "
@@ -4584,7 +4578,6 @@ static bool combineInstructionsOverFunction(
DominatorTree &DT, OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
ProfileSummaryInfo *PSI, unsigned MaxIterations, LoopInfo *LI) {
auto &DL = F.getParent()->getDataLayout();
- MaxIterations = std::min(MaxIterations, LimitMaxIterations.getValue());
/// Builder - This is an IRBuilder that automatically inserts new
/// instructions into the worklist when they are created.
@@ -4646,10 +4639,17 @@ static bool combineInstructionsOverFunction(
return MadeIRChange;
}
-InstCombinePass::InstCombinePass() : MaxIterations(LimitMaxIterations) {}
+InstCombinePass::InstCombinePass(InstCombineOptions Opts) : Options(Opts) {}
-InstCombinePass::InstCombinePass(unsigned MaxIterations)
- : MaxIterations(MaxIterations) {}
+void InstCombinePass::printPipeline(
+ raw_ostream &OS, function_ref<StringRef(StringRef)> MapClassName2PassName) {
+ static_cast<PassInfoMixin<InstCombinePass> *>(this)->printPipeline(
+ OS, MapClassName2PassName);
+ OS << '<';
+ OS << "max-iterations=" << Options.MaxIterations << ";";
+ OS << (Options.UseLoopInfo ? "" : "no-") << "use-loop-info";
+ OS << '>';
+}
PreservedAnalyses InstCombinePass::run(Function &F,
FunctionAnalysisManager &AM) {
@@ -4659,7 +4659,11 @@ PreservedAnalyses InstCombinePass::run(Function &F,
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
auto &TTI = AM.getResult<TargetIRAnalysis>(F);
+ // TODO: Only use LoopInfo when the option is set. This requires that the
+ // callers in the pass pipeline explicitly set the option.
auto *LI = AM.getCachedResult<LoopAnalysis>(F);
+ if (!LI && Options.UseLoopInfo)
+ LI = &AM.getResult<LoopAnalysis>(F);
auto *AA = &AM.getResult<AAManager>(F);
auto &MAMProxy = AM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
@@ -4669,7 +4673,7 @@ PreservedAnalyses InstCombinePass::run(Function &F,
&AM.getResult<BlockFrequencyAnalysis>(F) : nullptr;
if (!combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, DT, ORE,
- BFI, PSI, MaxIterations, LI))
+ BFI, PSI, Options.MaxIterations, LI))
// No changes, all analyses are preserved.
return PreservedAnalyses::all();
diff --git a/llvm/test/Other/new-pm-print-pipeline.ll b/llvm/test/Other/new-pm-print-pipeline.ll
index b0374f9978226..3ddc3698ea175 100644
--- a/llvm/test/Other/new-pm-print-pipeline.ll
+++ b/llvm/test/Other/new-pm-print-pipeline.ll
@@ -92,4 +92,8 @@
;; Test SeparateConstOffsetFromGEPPass option.
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='separate-const-offset-from-gep<lower-gep>' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-27
-; CHECK-27: function(separate-const-offset-from-gep<lower-gep>)
\ No newline at end of file
+; CHECK-27: function(separate-const-offset-from-gep<lower-gep>)
+
+;; Test InstCombine options - the first pass checks default settings, and the second checks customized options.
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(instcombine,instcombine<use-loop-info;max-iterations=42>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-28
+; CHECK-28: function(instcombine<max-iterations=1000;no-use-loop-info>,instcombine<max-iterations=42;use-loop-info>)
diff --git a/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll b/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll
index 89fb7d3ebed0a..351887afef9ea 100644
--- a/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll
+++ b/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll
@@ -1,5 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -opaque-pointers=0 < %s -passes='require<loops>,instcombine' -S | FileCheck %s
+; RUN: opt -opaque-pointers=0 < %s -passes='instcombine<use-loop-info>' -S | FileCheck %s
+
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/Transforms/InstCombine/statepoint-cleanup.ll b/llvm/test/Transforms/InstCombine/statepoint-cleanup.ll
index b79a7f3c75ad0..61c2b85553fd9 100644
--- a/llvm/test/Transforms/InstCombine/statepoint-cleanup.ll
+++ b/llvm/test/Transforms/InstCombine/statepoint-cleanup.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=instcombine -instcombine-max-iterations=1 -S | FileCheck %s
+; RUN: opt < %s -passes='instcombine<max-iterations=1>' -S | FileCheck %s
; These tests check the optimizations specific to
; pointers being relocated at a statepoint.
diff --git a/llvm/test/Transforms/InstCombine/statepoint-iter.ll b/llvm/test/Transforms/InstCombine/statepoint-iter.ll
index 91f61366a5d6a..7aced27c56be0 100644
--- a/llvm/test/Transforms/InstCombine/statepoint-iter.ll
+++ b/llvm/test/Transforms/InstCombine/statepoint-iter.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=instcombine -instcombine-max-iterations=1 -S | FileCheck %s
+; RUN: opt < %s -passes='instcombine<max-iterations=1>' -S | FileCheck %s
; These tests check the optimizations specific to
; pointers being relocated at a statepoint.
More information about the llvm-commits
mailing list