[VPlan] Fix first-order splices without header mask not using EVL #146672
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2114,21 +2114,19 @@ void VPlanTransforms::addActiveLaneMask( | |
| HeaderMask->replaceAllUsesWith(LaneMask); | ||
| } | ||
|
|
||
| /// Try to optimize a \p CurRecipe masked by \p HeaderMask to a corresponding | ||
| /// EVL-based recipe without the header mask. Returns nullptr if no EVL-based | ||
| /// recipe could be created. | ||
| /// \p HeaderMask Header Mask. | ||
| /// \p CurRecipe Recipe to be transform. | ||
| /// \p TypeInfo VPlan-based type analysis. | ||
| /// \p AllOneMask The vector mask parameter of vector-predication intrinsics. | ||
| /// \p EVL The explicit vector length parameter of vector-predication | ||
| /// intrinsics. | ||
| static VPRecipeBase *optimizeMaskToEVL(VPValue *HeaderMask, | ||
| VPRecipeBase &CurRecipe, | ||
| VPTypeAnalysis &TypeInfo, | ||
| VPValue &AllOneMask, VPValue &EVL) { | ||
| using namespace llvm::VPlanPatternMatch; | ||
| auto GetNewMask = [&](VPValue *OrigMask) -> VPValue * { | ||
| assert(OrigMask && "Unmasked recipe when folding tail"); | ||
|
|
@@ -2153,18 +2151,6 @@ static VPRecipeBase *createEVLRecipe(VPValue *HeaderMask, | |
| return new VPReductionEVLRecipe(*Red, EVL, NewMask); | ||
| }) | ||
| .Case<VPInstruction>([&](VPInstruction *VPI) -> VPRecipeBase * { | ||
| VPValue *LHS, *RHS; | ||
| // Transform select with a header mask condition | ||
| // select(header_mask, LHS, RHS) | ||
|
|
@@ -2197,9 +2183,12 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) { | |
| "User of VF that we can't transform to EVL."); | ||
| Plan.getVF().replaceAllUsesWith(&EVL); | ||
|
|
||
| // Defer erasing recipes till the end so that we don't invalidate the | ||
| // VPTypeAnalysis cache. | ||
| SmallVector<VPRecipeBase *> ToErase; | ||
|
|
||
| // Create a scalar phi to track the previous EVL if fixed-order recurrence is | ||
| // contained. | ||
| bool ContainsFORs = | ||
| any_of(Header->phis(), IsaPred<VPFirstOrderRecurrencePHIRecipe>); | ||
| if (ContainsFORs) { | ||
|
|
@@ -2212,16 +2201,37 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) { | |
| DebugLoc()); | ||
|
|
||
| Builder.setInsertPoint(Header, Header->getFirstNonPhi()); | ||
| VPValue *PrevEVL = | ||
| Builder.createScalarPhi({MaxEVL, &EVL}, DebugLoc(), "prev.evl"); | ||
|
|
||
| for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>( | ||
| vp_depth_first_deep(Plan.getVectorLoopRegion()->getEntry()))) { | ||
| for (VPRecipeBase &R : *VPBB) { | ||
| using namespace VPlanPatternMatch; | ||
| VPValue *V1, *V2; | ||
| if (!match(&R, | ||
| m_VPInstruction<VPInstruction::FirstOrderRecurrenceSplice>( | ||
| m_VPValue(V1), m_VPValue(V2)))) | ||
| continue; | ||
| VPValue *Imm = Plan.getOrAddLiveIn( | ||
| ConstantInt::getSigned(Type::getInt32Ty(Ctx), -1)); | ||
| VPWidenIntrinsicRecipe *VPSplice = new VPWidenIntrinsicRecipe( | ||
| Intrinsic::experimental_vp_splice, | ||
| {V1, V2, Imm, AllOneMask, PrevEVL, &EVL}, | ||
| TypeInfo.inferScalarType(R.getVPSingleValue()), R.getDebugLoc()); | ||
| VPSplice->insertBefore(&R); | ||
| R.getVPSingleValue()->replaceAllUsesWith(VPSplice); | ||
| ToErase.push_back(&R); | ||
| } | ||
| } | ||
|
Comment on lines
+2204
to
+2226
There was a problem hiding this comment. Could we extract the code into a static function? There was a problem hiding this comment. Which part needs extracted? The loop over each recipe? There was a problem hiding this comment. Extract if possible. There was a problem hiding this comment. Oh yeah, I had the same thoughts too. There's a lot of places that do this in VPlanTransforms. We can look at doing this in a separate NFC patch? There was a problem hiding this comment. Btw I've gone ahead and merged this to fix the EVL buildbot, hope this wasn't too hasty. Happy to address any more review comments if you have them! |
||
| } | ||
|
|
||
| // Try to optimize header mask recipes away to their EVL variants. | ||
| for (VPValue *HeaderMask : collectAllHeaderMasks(Plan)) { | ||
| for (VPUser *U : collectUsersRecursively(HeaderMask)) { | ||
| auto *CurRecipe = cast<VPRecipeBase>(U); | ||
| VPRecipeBase *EVLRecipe = | ||
| optimizeMaskToEVL(HeaderMask, *CurRecipe, TypeInfo, *AllOneMask, EVL); | ||
| if (!EVLRecipe) | ||
| continue; | ||
|
|
||
|
|
@@ -2237,8 +2247,6 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) { | |
| VPValue *CurVPV = CurRecipe->getVPSingleValue(); | ||
| CurVPV->replaceAllUsesWith(EVLRecipe->getVPSingleValue()); | ||
| } | ||
| ToErase.push_back(CurRecipe); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Instead of first checking if there are any first-order recurrence phis and then iterating over all recipes in the loop region, can we instead just iterate over all header phis , and simply iterate over the users to replace the splices?
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 think so, all splices need to have a first-order recurrence phi as an operand right? Will take a look and submit a follow up PR