From d3f62b22e40c7ed0b75de03169dfbd75fa255df6 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Mon, 13 Jan 2025 17:32:11 +0800 Subject: [PATCH 1/2] [VPlan] Verify plan before optimizations. NFC I've been exploring verifying the VPlan before and after the EVL transformation steps, and noticed that the VPlan comes out in an invalid state between construction and optimisation. In adjustRecipesForReductions, we leave behind some dead recipes which are invalid: 1) When we replace a link with a reduction recipe, the old link ends up becoming a use-before-def: WIDEN ir<%l7> = add ir<%sum.02>, ir<%indvars.iv>.1 WIDEN ir<%l8> = add ir<%l7>.1, ir<%l3> WIDEN ir<%l9> = add ir<%l8>.1, ir<%l5> ... REDUCE ir<%l7>.1 = ir<%sum.02> + reduce.add (ir<%indvars.iv>.1) REDUCE ir<%l8>.1 = ir<%l7>.1 + reduce.add (ir<%l3>) REDUCE ir<%l9>.1 = ir<%l8>.1 + reduce.add (ir<%l5>) 2) When transforming an AnyOf reduction phi to a boolean, we leave behind a select with mismatching operand types, which triggers the assertions in VTypeAnalysis This adds an extra verification step and deletes the dead recipes eagerly to keep the plan valid. --- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp index d32a463a996c4..5bd4ae5e0a442 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -8837,6 +8837,8 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF, for (ElementCount VF = MinVF; ElementCount::isKnownLT(VF, MaxVFTimes2);) { VFRange SubRange = {VF, MaxVFTimes2}; if (auto Plan = tryToBuildVPlanWithVPRecipes(SubRange)) { + assert(verifyVPlanIsValid(*Plan) && "VPlan is invalid"); + // Now optimize the initial VPlan. if (!Plan->hasVF(ElementCount::getFixed(1))) VPlanTransforms::truncateToMinimalBitwidths(*Plan, @@ -9553,6 +9555,8 @@ void LoopVectorizationPlanner::adjustRecipesForReductions( VPRegionBlock *VectorLoopRegion = Plan->getVectorLoopRegion(); VPBasicBlock *Header = VectorLoopRegion->getEntryBasicBlock(); VPBasicBlock *MiddleVPBB = Plan->getMiddleBlock(); + SmallVector ToDelete; + for (VPRecipeBase &R : Header->phis()) { auto *PhiR = dyn_cast(&R); if (!PhiR || !PhiR->isInLoop() || (MinVF.isScalar() && !PhiR->isOrdered())) @@ -9672,10 +9676,11 @@ void LoopVectorizationPlanner::adjustRecipesForReductions( CM.useOrderedReductions(RdxDesc), CurrentLinkI->getDebugLoc()); // Append the recipe to the end of the VPBasicBlock because we need to // ensure that it comes after all of it's inputs, including CondOp. - // Note that this transformation may leave over dead recipes (including - // CurrentLink), which will be cleaned by a later VPlan transform. + // Delete CurrentLink as it will be invalid if its operand is replaced + // with a reduction defined at the bottom of the block in the next link. LinkVPBB->appendRecipe(RedRecipe); CurrentLink->replaceAllUsesWith(RedRecipe); + ToDelete.push_back(CurrentLink); PreviousLink = RedRecipe; } } @@ -9790,6 +9795,8 @@ void LoopVectorizationPlanner::adjustRecipesForReductions( Cmp = Builder.createNot(Cmp); VPValue *Or = Builder.createOr(PhiR, Cmp); Select->getVPSingleValue()->replaceAllUsesWith(Or); + // Delete Select now that it has invalid types. + ToDelete.push_back(Select); // Convert the reduction phi to operate on bools. PhiR->setOperand(0, Plan->getOrAddLiveIn(ConstantInt::getFalse( @@ -9807,6 +9814,8 @@ void LoopVectorizationPlanner::adjustRecipesForReductions( } VPlanTransforms::clearReductionWrapFlags(*Plan); + for (VPRecipeBase *R : ToDelete) + R->eraseFromParent(); } void VPDerivedIVRecipe::execute(VPTransformState &State) { From 3f152458857de1ac8350c283930427165e7011b0 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Mon, 13 Jan 2025 18:23:52 +0800 Subject: [PATCH 2/2] Move verification to tryToBuildVPlanWithVPRecipes --- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp index 5bd4ae5e0a442..e9af2dd32fdba 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -8837,8 +8837,6 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF, for (ElementCount VF = MinVF; ElementCount::isKnownLT(VF, MaxVFTimes2);) { VFRange SubRange = {VF, MaxVFTimes2}; if (auto Plan = tryToBuildVPlanWithVPRecipes(SubRange)) { - assert(verifyVPlanIsValid(*Plan) && "VPlan is invalid"); - // Now optimize the initial VPlan. if (!Plan->hasVF(ElementCount::getFixed(1))) VPlanTransforms::truncateToMinimalBitwidths(*Plan, @@ -9482,6 +9480,8 @@ LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) { VPlanTransforms::addActiveLaneMask(*Plan, ForControlFlow, WithoutRuntimeCheck); } + + assert(verifyVPlanIsValid(*Plan) && "VPlan is invalid"); return Plan; }