[LoopVectorize] Vectorize fixed-order recurrence with vscale x 1. #142772
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6139,11 +6139,6 @@ LoopVectorizationCostModel::getInstructionCost(Instruction *I, | |
|
|
||
| // First-order recurrences are replaced by vector shuffles inside the loop. | ||
| if (VF.isVector() && Legal->isFixedOrderRecurrence(Phi)) { | ||
| SmallVector<int> Mask(VF.getKnownMinValue()); | ||
| std::iota(Mask.begin(), Mask.end(), VF.getKnownMinValue() - 1); | ||
| return TTI.getShuffleCost(TargetTransformInfo::SK_Splice, | ||
|
|
@@ -8561,13 +8556,17 @@ addUsersInExitBlocks(VPlan &Plan, | |
| /// users in the original exit block using the VPIRInstruction wrapping to the | ||
| /// LCSSA phi. | ||
| static void addExitUsersForFirstOrderRecurrences( | ||
| VPlan &Plan, SetVector<VPIRInstruction *> &ExitUsersToFix, VFRange &Range) { | ||
| VPRegionBlock *VectorRegion = Plan.getVectorLoopRegion(); | ||
| auto *ScalarPHVPBB = Plan.getScalarPreheader(); | ||
| auto *MiddleVPBB = Plan.getMiddleBlock(); | ||
| VPBuilder ScalarPHBuilder(ScalarPHVPBB); | ||
| VPBuilder MiddleBuilder(MiddleVPBB, MiddleVPBB->getFirstNonPhi()); | ||
|
|
||
| auto IsScalableOne = [](ElementCount VF) -> bool { | ||
| return VF == ElementCount::getScalable(1); | ||
| }; | ||
|
|
||
| for (auto &HeaderPhi : VectorRegion->getEntryBasicBlock()->phis()) { | ||
| auto *FOR = dyn_cast<VPFirstOrderRecurrencePHIRecipe>(&HeaderPhi); | ||
| if (!FOR) | ||
|
|
@@ -8649,6 +8648,13 @@ static void addExitUsersForFirstOrderRecurrences( | |
| for (VPIRInstruction *ExitIRI : ExitUsersToFix) { | ||
| if (ExitIRI->getOperand(0) != FOR) | ||
| continue; | ||
| // For VF vscale x 1, if vscale = 1, we are unable to extract the | ||
| // penultimate value of the recurrence. Instead, we can extract the last | ||
| // element directly from VPInstruction::FirstOrderRecurrenceSplice. | ||
|
Comment on lines
+8651
to
+8653
There was a problem hiding this comment. Sounds somewhat similar to noted (exactly a year ago) in As noted in #137030 (comment), There was a problem hiding this comment.
Yes, it will work well for all target. We're trying to enable tail folding (specifically EVL tail folding) when a fixed-order recurrence (FOR) phi is live-out from the loop. In this case, all live-out values of the FOR phi must be obtained by extracting the last active lane from the recurrence phi.
Yes, I also left a TODO comment for UF because we currently don’t have a method to clamp the VPlan for UF. |
||
| // TODO: Consider vscale_range info and UF. | ||
| if (LoopVectorizationPlanner::getDecisionAndClampRange(IsScalableOne, | ||
| Range)) | ||
| return; | ||
| VPValue *PenultimateElement = MiddleBuilder.createNaryOp( | ||
| VPInstruction::ExtractPenultimateElement, {FOR->getBackedgeValue()}, | ||
| {}, "vector.recur.extract.for.phi"); | ||
|
|
@@ -8864,7 +8870,7 @@ VPlanPtr LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes( | |
| addScalarResumePhis(RecipeBuilder, *Plan, IVEndValues); | ||
| SetVector<VPIRInstruction *> ExitUsersToFix = | ||
| collectUsersInLatchExitBlock(*Plan); | ||
| addExitUsersForFirstOrderRecurrences(*Plan, ExitUsersToFix, Range); | ||
| addUsersInExitBlocks(*Plan, ExitUsersToFix); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3850,9 +3850,6 @@ VPFirstOrderRecurrencePHIRecipe::computeCost(ElementCount VF, | |
| if (VF.isScalar()) | ||
| return Ctx.TTI.getCFInstrCost(Instruction::PHI, Ctx.CostKind); | ||
|
|
||
|
Comment on lines
-3853
to
-3855
There was a problem hiding this comment. Would it be possible to add a target-specific test case, where the cost model chooses vscale x 1, to make sure all cost model code paths are exercised? There was a problem hiding this comment. Sure, I think you're referring to this. There was a problem hiding this comment. Perhaps I'm wrong, but I think @fhahn means a test where we don't specify -force-vector-width=1 and the cost model ends up choosing There was a problem hiding this comment. I tried it, but without specifying VF, the current upstream RISC-V cost model will select vscale x 2 for this case. There was a problem hiding this comment. Hmm, I guess it's tricky to write a test for this. I think one way of doing it is to introduce a possible dependency between reads and writes in the loop such that we are forced to limit the VF to vscale x 1 using a vscale_range of (1,16) or something like that. The only problem then would be the vectoriser may just prefer a fixed-width VF instead, i.e. VF=4. There was a problem hiding this comment. Perhaps you could have a loop with a combination of gathers and normal loads/stores with a dependency issue that limits the VF to vscale x 1. Requiring gather loads may then favour scalable vectors? |
||
| return 0; | ||
| } | ||
|
|
||
|
|
||
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.
Does this imply that
VF.isVector()returns true for single-element non-vector VF's when VF.isScalable() && VF.getKnownMinValue() == 1 and vscale == 1? The TODO is erased w/o considering vscale_range info?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.
47c9260
This TODO should be kept.
Mel-Chen@d63194a
I tried tightening the condition for clamping the VF range, but it seems to get stuck on the design of ExtractLastElement and VPLane.
I removed the assertion as work-around way. While it appears to produce correct IR, I believe there must be a better solution.