-
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 5 commits
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 |
|---|---|---|
|
|
@@ -890,6 +890,17 @@ static void simplifyRecipe(VPRecipeBase &R, VPTypeAnalysis &TypeInfo) { | |
| return; | ||
| } | ||
|
|
||
| // VPScalarIVSteps can only be simplified after unrolling. VPScalarIVSteps for | ||
| // part 0 can be replaced by their start value, if only the first lane is | ||
| // demanded. | ||
| 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->isPart0() && | ||
|
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. If the plan has been unrolled can this even be part > 0? I just wonder if instead you can simply assert
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. In general it is the other way around, unrolling introduces copies of the input recipe, one for each part. So only after unrolling recipes for different 'parts' (i.e. interleaved iterations) are introduced. |
||
| 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(); | ||
|
|
@@ -966,7 +977,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); | ||
|
|
@@ -1043,7 +1054,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.
Should this return a
bool?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.
Fixed, thanks