-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[VPlan] Remove no-op SCALAR-STEPS after unrolling. #123655
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
Changes from 1 commit
08eb47d
524b204
1202d5f
8f5a479
6b2a4bd
d704a98
a9110a3
68e10bd
49c1b87
08ad022
d9bfec8
4120bd3
9733017
08b3f2b
0aa5d80
6146511
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4026,6 +4026,9 @@ class VPlan { | |
| UFs.insert(UF); | ||
| } | ||
|
|
||
| /// Returns true if the VPlan already has been unrolled, i.e. it has UF = 1. | ||
| unsigned isUnrolled() const { return UFs.size() == 1 && UFs.back() == 1; } | ||
|
||
|
|
||
| /// Return a string with the name of the plan and the applicable VFs and UFs. | ||
| std::string getName() const; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -888,6 +888,14 @@ static void simplifyRecipe(VPRecipeBase &R, VPTypeAnalysis &TypeInfo) { | |
| return; | ||
| } | ||
|
|
||
| if (auto *Steps = dyn_cast<VPScalarIVStepsRecipe>(&R)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @fhahn, what I meant in one of my earlier comments was about taking everything in this patch except this new
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep I was just working on that after updating to latest main, but got sidetracked before I could put up the PR. It's here now: #125926
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. VPWidenEVL is gone now, so I removed the changes to VPlanPatternMatch from the patch again |
||
| if (Steps->getParent()->getPlan()->isUnrolled() && | ||
| Steps->getNumOperands() == 2 && vputils::onlyFirstLaneUsed(Steps)) { | ||
|
||
| Steps->replaceAllUsesWith(Steps->getOperand(0)); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| VPValue *A; | ||
| if (match(&R, m_Trunc(m_ZExtOrSExt(m_VPValue(A))))) { | ||
| VPValue *Trunc = R.getVPSingleValue(); | ||
|
|
@@ -964,7 +972,7 @@ static void simplifyRecipe(VPRecipeBase &R, VPTypeAnalysis &TypeInfo) { | |
|
|
||
| /// Try to simplify the recipes in \p Plan. Use \p CanonicalIVTy as type for all | ||
| /// un-typed live-ins in VPTypeAnalysis. | ||
| static void simplifyRecipes(VPlan &Plan, Type *CanonicalIVTy) { | ||
| void VPlanTransforms::simplifyRecipes(VPlan &Plan, Type *CanonicalIVTy) { | ||
| ReversePostOrderTraversal<VPBlockDeepTraversalWrapper<VPBlockBase *>> RPOT( | ||
| Plan.getEntry()); | ||
| VPTypeAnalysis TypeInfo(CanonicalIVTy); | ||
|
|
@@ -1041,7 +1049,6 @@ void VPlanTransforms::optimizeForVFAndUF(VPlan &Plan, ElementCount BestVF, | |
| } | ||
|
|
||
| Term->eraseFromParent(); | ||
| VPlanTransforms::removeDeadRecipes(Plan); | ||
|
|
||
| Plan.setVF(BestVF); | ||
| Plan.setUF(BestUF); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the code to refactor simplifyRecipes could actually be its own NFC patch, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean just exposing it in VPlanTransforms.h without adding new users outside of VPlanTransforms.cpp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just meant the changes to pull out the
removeDeadRecipescall fromsimplifyRecipesand callsimplifyRecipesandremoveDeadRecipeshere fromexecutePlan. It looked like these changes are notionally independent from your changes insimplifyRecipeforVPScalarIVStepsRecipe