From 4ecc6af813e2568c3529ea0fe55eaea891d8ab49 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Fri, 17 Feb 2023 10:11:40 -0500 Subject: [PATCH] [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 --- .../llvm/Transforms/InstCombine/InstCombine.h | 28 +++++++++++++++++++--- llvm/lib/Passes/PassBuilder.cpp | 27 +++++++++++++++++++++ llvm/lib/Passes/PassRegistry.def | 9 +++++++ .../InstCombine/InstructionCombining.cpp | 26 +++++++++++--------- llvm/test/Other/new-pm-print-pipeline.ll | 6 ++++- .../InstCombine/gep-combine-loop-invariant.ll | 3 ++- .../Transforms/InstCombine/statepoint-cleanup.ll | 2 +- .../test/Transforms/InstCombine/statepoint-iter.ll | 2 +- 8 files changed, 85 insertions(+), 18 deletions(-) diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombine.h b/llvm/include/llvm/Transforms/InstCombine/InstCombine.h index 35a3a8c..9c6bcc6 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 { +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 MapClassName2PassName); PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM); }; diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index dcd002d..e335e51 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -778,6 +778,33 @@ Expected parseSimplifyCFGOptions(StringRef Params) { return Result; } +Expected 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( + formatv("invalid argument to InstCombine pass max-iterations " + "parameter: '{0}' ", + ParamName).str(), + inconvertibleErrorCode()); + Result.setMaxIterations((unsigned)MaxIterations.getZExtValue()); + } else { + return make_error( + formatv("invalid InstCombine pass parameter '{0}' ", ParamName).str(), + inconvertibleErrorCode()); + } + } + return Result; +} + /// Parser of parameters for LoopVectorize pass. Expected parseLoopVectorizeOptions(StringRef Params) { LoopVectorizeOptions Opts; diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def index 0412691..63b3983 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 54392bb..4f8ea32 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 MaxSinkNumUsers( "instcombine-max-sink-users", cl::init(32), cl::desc("Maximum number of undroppable users for instruction sinking")); -static cl::opt LimitMaxIterations( - "instcombine-max-iterations", - cl::desc("Limit the maximum number of instruction combining iterations"), - cl::init(InstCombineDefaultMaxIterations)); - static cl::opt 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 MapClassName2PassName) { + static_cast *>(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(F); auto &TTI = AM.getResult(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(F); + if (!LI && Options.UseLoopInfo) + LI = &AM.getResult(F); auto *AA = &AM.getResult(F); auto &MAMProxy = AM.getResult(F); @@ -4669,7 +4673,7 @@ PreservedAnalyses InstCombinePass::run(Function &F, &AM.getResult(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 b0374f9..3ddc369 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' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-27 -; CHECK-27: function(separate-const-offset-from-gep) \ No newline at end of file +; CHECK-27: function(separate-const-offset-from-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)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-28 +; CHECK-28: function(instcombine,instcombine) diff --git a/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll b/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll index 89fb7d3..351887a 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,instcombine' -S | FileCheck %s +; RUN: opt -opaque-pointers=0 < %s -passes='instcombine' -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 b79a7f3..61c2b85 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' -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 91f6136..7aced27 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' -S | FileCheck %s ; These tests check the optimizations specific to ; pointers being relocated at a statepoint. -- 2.7.4