[LV][NFC] Print VPlan's transformation pipline

[nikolaypanchenko]

Print VPlan before each pass in VPlanTransforms::optimize with a debug type vplan-xforms.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Kolya Panchenko (nikolaypanchenko)

Changes

Print VPlan before each pass in VPlanTransforms::optimize with a debug type vplan-xforms.

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+25)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp index 0eaaa037ad5782f..142cda07adbd763 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp @@ -24,6 +24,8 @@ #include "llvm/IR/Intrinsics.h" #include "llvm/IR/PatternMatch.h" +#define DEBUG_TYPE "vplan-xforms" + using namespace llvm; using namespace llvm::PatternMatch; @@ -871,16 +873,39 @@ static void simplifyRecipes(VPlan &Plan, LLVMContext &Ctx) { } void VPlanTransforms::optimize(VPlan &Plan, ScalarEvolution &SE) { + LLVM_DEBUG(dbgs() << "LV: --- VPlan before removeRedundantCanonicalIVs ---\n"; + Plan.dump(); dbgs() << "\n\n"); removeRedundantCanonicalIVs(Plan); + + LLVM_DEBUG( + dbgs() << "LV: --- VPlan before removeRedundantInductionCasts ---\n"; + Plan.dump(); dbgs() << "\n\n"); removeRedundantInductionCasts(Plan); + LLVM_DEBUG(dbgs() << "LV: --- VPlan before optimizeInductions ---\n"; + Plan.dump(); dbgs() << "\n\n"); optimizeInductions(Plan, SE); + + LLVM_DEBUG(dbgs() << "LV: --- VPlan before simplifyRecipes ---\n"; + Plan.dump(); dbgs() << "\n\n"); simplifyRecipes(Plan, SE.getContext()); + + LLVM_DEBUG(dbgs() << "LV: --- VPlan before removeDeadRecipes ---\n"; + Plan.dump(); dbgs() << "\n\n"); removeDeadRecipes(Plan); + LLVM_DEBUG( + dbgs() << "LV: --- VPlan before createAndOptimizeReplicateRegions ---\n"; + Plan.dump(); dbgs() << "\n\n"); createAndOptimizeReplicateRegions(Plan); + LLVM_DEBUG( + dbgs() << "LV: --- VPlan before removeRedundantExpandSCEVRecipes ---\n"; + Plan.dump(); dbgs() << "\n\n"); removeRedundantExpandSCEVRecipes(Plan); + + LLVM_DEBUG(dbgs() << "LV: --- VPlan before mergeBlocksIntoPredecessors ---\n"; + Plan.dump(); dbgs() << "\n\n"); mergeBlocksIntoPredecessors(Plan); } 

[fhahn]

Thanks for working on this functionality, should be very helpful in the future.

I think it would be good to have a generic way to register a pass to run, so we have a single place to add the print statements. Eg have a RUN_PASS macro or helper function that takes the function to run plus a name perhaps (or just uses the function name).

Not sure about tying this to debug output, as this potentially adds a lot of output. It might be better to have this controlled by a separate -vplan-print-before-all? Or even better to update all transforms to indicate whether they made changes and have a more compact -vplan-print-before-changed

[nikolaypanchenko]

Thanks for working on this functionality, should be very helpful in the future.

I think it would be good to have a generic way to register a pass to run, so we have a single place to add the print statements. Eg have a RUN_PASS macro or helper function that takes the function to run plus a name perhaps (or just uses the function name).

Not sure about tying this to debug output, as this potentially adds a lot of output. It might be better to have this controlled by a separate -vplan-print-before-all? Or even better to update all transforms to indicate whether they made changes and have a more compact -vplan-print-before-changed

Thanks for working on this functionality, should be very helpful in the future.

I think it would be good to have a generic way to register a pass to run, so we have a single place to add the print statements. Eg have a RUN_PASS macro or helper function that takes the function to run plus a name perhaps (or just uses the function name).

Sure, I can build VPlanPass and VPlanPassManager. At this point they don't need to be as advanced as LLVM's or MLIR's pass/passmanager, but they can evolve later

Not sure about tying this to debug output, as this potentially adds a lot of output. It might be better to have this controlled by a separate -vplan-print-before-all? Or even better to update all transforms to indicate whether they made changes and have a more compact -vplan-print-before-changed

I was thinking about that too and tried to minimize changes and impact on a loop-vectorize debug output, so with the changeset extra output will only appear with vplan-xforms debug type.

[fhahn]

Sure, I can build VPlanPass and VPlanPassManager. At this point they don't need to be as advanced as LLVM's or MLIR's pass/passmanager, but they can evolve later

I don't think we need to add all the extra structure & complexity yet; runner macro or function taking a function_ref would suffice?

I was thinking about that too and tried to minimize changes and impact on a loop-vectorize debug output, so with the changeset extra output will only appear with vplan-xforms debug type.

I was thinking about cases where people just use -debug (in tests and while debugging issues), which will get the full output

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 8cd7184dc0f1e5e28e39b03c90bc9aeee96d104f eb7125b99b921fc19ad736fa42150266b4ba2e01 -- llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp llvm/lib/Transforms/Vectorize/VPlanTransforms.h

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp index 4d55669648..4bb0e88e57 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp @@ -24,7 +24,6 @@ #include "llvm/IR/Intrinsics.h" #include "llvm/IR/PatternMatch.h" - using namespace llvm; static cl::list<std::string> @@ -37,12 +36,14 @@ static cl::list<std::string> llvm::cl::desc("Print VPlan after specified VPlan passes"), cl::CommaSeparated, cl::Hidden); -static cl::opt<bool> PrintBeforeAll("vplan-print-before-all", - llvm::cl::desc("Print VPlan before each VPlan pass"), - cl::init(false), cl::Hidden); -static cl::opt<bool> PrintAfterAll("vplan-print-after-all", - llvm::cl::desc("Print VPlan after each VPlanpass"), - cl::init(false), cl::Hidden); +static cl::opt<bool> + PrintBeforeAll("vplan-print-before-all", + llvm::cl::desc("Print VPlan before each VPlan pass"), + cl::init(false), cl::Hidden); +static cl::opt<bool> + PrintAfterAll("vplan-print-after-all", + llvm::cl::desc("Print VPlan after each VPlanpass"), + cl::init(false), cl::Hidden); using namespace llvm::PatternMatch; @@ -560,10 +561,10 @@ struct RemoveDeadRecipes : public VPlanPass<RemoveDeadRecipes> { } }; -static VPValue * -createScalarIVSteps(VPlan &Plan, const InductionDescriptor &ID, - ScalarEvolution &SE, Instruction *TruncI, Type *IVTy, - VPValue *StartV, VPValue *Step) { +static VPValue *createScalarIVSteps(VPlan &Plan, const InductionDescriptor &ID, + ScalarEvolution &SE, Instruction *TruncI, + Type *IVTy, VPValue *StartV, + VPValue *Step) { VPBasicBlock *HeaderVPBB = Plan.getVectorLoopRegion()->getEntryBasicBlock(); auto IP = HeaderVPBB->getFirstNonPhi(); VPCanonicalIVPHIRecipe *CanonicalIV = Plan.getCanonicalIV(); @@ -591,7 +592,9 @@ private: public: explicit OptimizeInductionsPass(ScalarEvolution &SE) : SE(SE) {} - virtual StringRef getPassArgument() const override { return "optimize-inductions"; } + virtual StringRef getPassArgument() const override { + return "optimize-inductions"; + } virtual void run(VPlan &Plan) override { SmallVector<VPRecipeBase *> ToRemove; 

[nikolaypanchenko]

I don't think we need to add all the extra structure & complexity yet; runner macro or function taking a function_ref would suffice?

I do agree that that is good simple approach. On the other hand to do some baby step towards to pass/passmanager for vectorizer might be useful. The last commit introduces simple base class for pass, wraps all existing xforms and copies existing logic of print-before/after* options. If that looks to much, I'm fine to go with much simpler option

[nikolaypanchenko]

@fhahn ping

[fhahn]

I do agree that that is good simple approach. On the other hand to do some baby step towards to pass/passmanager for vectorizer might be useful. The last commit introduces simple base class for pass, wraps all existing xforms and copies existing logic of print-before/after* options. If that looks to much, I'm fine to go with much simpler option

Thanks for the update! I think to start with a simpler option would be preferred.

[fhahn]

reverse ping :)