[VPlan] Convert to concrete recipes before dissolving loop regions. NFCI#141999
Conversation
After updating llvm#118638 on tip of tree, expanding VPWidenIntOrFpInductionRecipes fails because it needs the loop region to get the latch to insert the increment into: VPBasicBlock *ExitingBB = Plan->getVectorLoopRegion()->getExitingBasicBlock(); Builder.setInsertPoint(ExitingBB, ExitingBB->getTerminator()->getIterator()); auto *Next = Builder.createNaryOp(AddOp, {Prev, Inc}, Flags, WidenIVR->getDebugLoc(), "vec.ind.next"); However after llvm#117506, the region is dissolved so it doesn't work. This shuffles the dissolveLoopRegions steps to be after convertToConcreteRecipes so we can use the region when expanding VPWidenIntOrFpInductionRecipes
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Luke Lau (lukel97) ChangesAfter updating #118638 on tip of tree, expanding VPWidenIntOrFpInductionRecipes fails because it needs the loop region to get the latch to insert the increment into: However after #117506, the region is dissolved so it doesn't work. This shuffles the dissolveLoopRegions steps to be after convertToConcreteRecipes so we can use the region when expanding VPWidenIntOrFpInductionRecipes Full diff: https://github.com/llvm/llvm-project/pull/141999.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 90e224ea8f37a..6bd06410614de 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7568,14 +7568,14 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
TTI.getRegisterBitWidth(TargetTransformInfo::RGK_FixedWidthVector));
VPlanTransforms::removeDeadRecipes(BestVPlan);
+ VPlanTransforms::convertToConcreteRecipes(BestVPlan,
+ *Legal->getWidestInductionType());
// Retrieve and store the middle block before dissolving regions. Regions are
// dissolved after optimizing for VF and UF, which completely removes unneeded
// loop regions first.
VPBasicBlock *MiddleVPBB =
BestVPlan.getVectorLoopRegion() ? BestVPlan.getMiddleBlock() : nullptr;
VPlanTransforms::dissolveLoopRegions(BestVPlan);
- VPlanTransforms::convertToConcreteRecipes(BestVPlan,
- *Legal->getWidestInductionType());
// Perform the actual loop transformation.
VPTransformState State(&TTI, BestVF, LI, DT, ILV.AC, ILV.Builder, &BestVPlan,
OrigLoop->getParentLoop(),
|
fhahn
left a comment
There was a problem hiding this comment.
That should be fine,. but currently there is a ordering constraint between convertToConcreteRecipes and dissolveLoopRegions, as replacing the CanIV before removing the region makes the region invalid (althoug just temporarily).
We should probably remove the canonical IV together with removing its region: #142372
That resolves the ordering issue
|
Post commit comment: the latch could be retrieved w/ or w/o regions similar to retrieving the header via |
After updating #118638 on tip of tree, expanding VPWidenIntOrFpInductionRecipes fails because it needs the loop region to get the latch to insert the increment into:
However after #117506, the region is dissolved so it doesn't work.
This shuffles the dissolveLoopRegions steps to be after convertToConcreteRecipes so we can use the region when expanding VPWidenIntOrFpInductionRecipes