Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nikolaypanchenko
Copy link
Contributor

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

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 17, 2023

@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);
 }
 

@npanchen npanchen requested a review from fhahn November 17, 2023 15:39
@fhahn fhahn assigned ayalz and aniragil and unassigned ayalz and aniragil Nov 20, 2023
@fhahn fhahn requested review from ayalz and aniragil November 20, 2023 15:25
@fhahn
Copy link
Contributor

fhahn commented Nov 20, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor

fhahn commented Nov 21, 2023

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

Copy link

github-actions bot commented Nov 27, 2023

⚠️ 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;

Introduced `VPlanPass` as a base-class for transformations on a VPlan.
The class itself is extremely simple and has only 3 methods:
- `run(VPlan)`: executes the pass on a given `VPlan`
- `getName`: returns a name of the pass. Used to print `VPlan`
- `getPassArgument`: returns a name of the command line option to use in
  `vplan-print-[before|after]=<name>`

Wrapped all existing transformations of `VPlanTransforms` with that new
class and added convenient function to run the pass and print `VPlan`
before/after it, i.e. that functionality is similar to LLVM's
`print-before*` and `print-after*` options.
@nikolaypanchenko
Copy link
Contributor Author

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
Copy link
Contributor Author

@fhahn ping

@fhahn
Copy link
Contributor

fhahn commented Jan 8, 2024

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
Copy link
Contributor

fhahn commented Feb 29, 2024

reverse ping :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants