[VPlan] Simplify branch on False in VPlan transform (NFC). #140409
Conversation
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
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-backend-powerpc Author: Florian Hahn (fhahn) ChangesSimplify branch on false, starting with the branch from the middle block Depends on #140405. Patch is 101.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140409.diff 32 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 148b187f4f25d..6c3a2ce941c54 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -490,7 +490,7 @@ class InnerLoopVectorizer {
MinProfitableTripCount(MinProfitableTripCount), UF(UnrollFactor),
Builder(PSE.getSE()->getContext()), Cost(CM), BFI(BFI), PSI(PSI),
RTChecks(RTChecks), Plan(Plan),
- VectorPHVPB(Plan.getEntry()->getSingleSuccessor()) {}
+ VectorPHVPB(Plan.getEntry()->getSuccessors()[1]) {}
virtual ~InnerLoopVectorizer() = default;
@@ -2365,22 +2365,19 @@ InnerLoopVectorizer::getOrCreateVectorTripCount(BasicBlock *InsertBlock) {
void InnerLoopVectorizer::introduceCheckBlockInVPlan(BasicBlock *CheckIRBB) {
VPBlockBase *ScalarPH = Plan.getScalarPreheader();
VPBlockBase *PreVectorPH = VectorPHVPB->getSinglePredecessor();
- if (PreVectorPH->getNumSuccessors() != 1) {
- assert(PreVectorPH->getNumSuccessors() == 2 && "Expected 2 successors");
- assert(PreVectorPH->getSuccessors()[0] == ScalarPH &&
- "Unexpected successor");
- VPIRBasicBlock *CheckVPIRBB = Plan.createVPIRBasicBlock(CheckIRBB);
- VPBlockUtils::insertOnEdge(PreVectorPH, VectorPHVPB, CheckVPIRBB);
- PreVectorPH = CheckVPIRBB;
- }
+ assert(PreVectorPH->getNumSuccessors() == 2 && "Expected 2 successors");
+ assert(PreVectorPH->getSuccessors()[0] == ScalarPH && "Unexpected successor");
+ VPIRBasicBlock *CheckVPIRBB = Plan.createVPIRBasicBlock(CheckIRBB);
+ VPBlockUtils::insertOnEdge(PreVectorPH, VectorPHVPB, CheckVPIRBB);
+ PreVectorPH = CheckVPIRBB;
VPBlockUtils::connectBlocks(PreVectorPH, ScalarPH);
PreVectorPH->swapSuccessors();
// We just connected a new block to the scalar preheader. Update all
// ResumePhis by adding an incoming value for it, replicating the last value.
for (VPRecipeBase &R : *cast<VPBasicBlock>(ScalarPH)) {
- auto *ResumePhi = dyn_cast<VPInstruction>(&R);
- if (!ResumePhi || ResumePhi->getOpcode() != VPInstruction::ResumePhi)
+ auto *ResumePhi = cast<VPPhi>(&R);
+ if (ResumePhi->getNumIncoming() == ScalarPH->getNumPredecessors())
continue;
ResumePhi->addOperand(
ResumePhi->getOperand(ResumePhi->getNumOperands() - 1));
@@ -2463,9 +2460,6 @@ void InnerLoopVectorizer::emitIterationCountCheck(BasicBlock *Bypass) {
setBranchWeights(BI, MinItersBypassWeights, /*IsExpected=*/false);
ReplaceInstWithInst(TCCheckBlock->getTerminator(), &BI);
LoopBypassBlocks.push_back(TCCheckBlock);
-
- // TODO: Wrap LoopVectorPreHeader in VPIRBasicBlock here.
- introduceCheckBlockInVPlan(TCCheckBlock);
}
BasicBlock *InnerLoopVectorizer::emitSCEVChecks(BasicBlock *Bypass) {
@@ -2530,7 +2524,8 @@ BasicBlock *InnerLoopVectorizer::emitMemRuntimeChecks(BasicBlock *Bypass) {
static void replaceVPBBWithIRVPBB(VPBasicBlock *VPBB, BasicBlock *IRBB) {
VPIRBasicBlock *IRVPBB = VPBB->getPlan()->createVPIRBasicBlock(IRBB);
for (auto &R : make_early_inc_range(*VPBB)) {
- assert(!R.isPhi() && "Tried to move phi recipe to end of block");
+ assert((IRVPBB->empty() || IRVPBB->back().isPhi() || !R.isPhi()) &&
+ "Tried to move phi recipe to end of block");
R.moveBefore(*IRVPBB, IRVPBB->end());
}
@@ -7768,10 +7763,7 @@ static void fixReductionScalarResumeWhenVectorizingEpilog(
// created a bc.merge.rdx Phi after the main vector body. Ensure that we carry
// over the incoming values correctly.
using namespace VPlanPatternMatch;
- auto IsResumePhi = [](VPUser *U) {
- auto *VPI = dyn_cast<VPInstruction>(U);
- return VPI && VPI->getOpcode() == VPInstruction::ResumePhi;
- };
+ auto IsResumePhi = [](VPUser *U) { return isa<VPPhi>(U); };
assert(count_if(EpiRedResult->users(), IsResumePhi) == 1 &&
"ResumePhi must have a single user");
auto *EpiResumePhiVPI =
@@ -7837,7 +7829,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
// 1. Set up the skeleton for vectorization, including vector pre-header and
// middle block. The vector loop is created during VPlan execution.
- VPBasicBlock *VectorPH = cast<VPBasicBlock>(Entry->getSingleSuccessor());
+ VPBasicBlock *VectorPH = cast<VPBasicBlock>(Entry->getSuccessors()[1]);
State.CFG.PrevBB = ILV.createVectorizedLoopSkeleton();
if (VectorizingEpilogue)
VPlanTransforms::removeDeadRecipes(BestVPlan);
@@ -8070,7 +8062,8 @@ EpilogueVectorizerMainLoop::emitIterationCountCheck(BasicBlock *Bypass,
setBranchWeights(BI, MinItersBypassWeights, /*IsExpected=*/false);
ReplaceInstWithInst(TCCheckBlock->getTerminator(), &BI);
- introduceCheckBlockInVPlan(TCCheckBlock);
+ if (!ForEpilogue)
+ introduceCheckBlockInVPlan(TCCheckBlock);
return TCCheckBlock;
}
@@ -8200,7 +8193,6 @@ EpilogueVectorizerEpilogueLoop::emitMinimumVectorEpilogueIterCountCheck(
Plan.setEntry(NewEntry);
// OldEntry is now dead and will be cleaned up when the plan gets destroyed.
- introduceCheckBlockInVPlan(Insert);
return Insert;
}
@@ -9146,9 +9138,8 @@ static VPInstruction *addResumePhiRecipeForInduction(
WideIV->getDebugLoc());
}
- auto *ResumePhiRecipe =
- ScalarPHBuilder.createNaryOp(VPInstruction::ResumePhi, {EndValue, Start},
- WideIV->getDebugLoc(), "bc.resume.val");
+ auto *ResumePhiRecipe = ScalarPHBuilder.createScalarPhi(
+ {EndValue, Start}, WideIV->getDebugLoc(), "bc.resume.val");
return ResumePhiRecipe;
}
@@ -9160,7 +9151,7 @@ static void addScalarResumePhis(VPRecipeBuilder &Builder, VPlan &Plan,
DenseMap<VPValue *, VPValue *> &IVEndValues) {
VPTypeAnalysis TypeInfo(Plan.getCanonicalIV()->getScalarType());
auto *ScalarPH = Plan.getScalarPreheader();
- auto *MiddleVPBB = cast<VPBasicBlock>(ScalarPH->getSinglePredecessor());
+ auto *MiddleVPBB = cast<VPBasicBlock>(ScalarPH->getPredecessors()[0]);
VPRegionBlock *VectorRegion = Plan.getVectorLoopRegion();
VPBuilder VectorPHBuilder(
cast<VPBasicBlock>(VectorRegion->getSinglePredecessor()));
@@ -9177,8 +9168,7 @@ static void addScalarResumePhis(VPRecipeBuilder &Builder, VPlan &Plan,
if (VPInstruction *ResumePhi = addResumePhiRecipeForInduction(
WideIVR, VectorPHBuilder, ScalarPHBuilder, TypeInfo,
&Plan.getVectorTripCount())) {
- assert(ResumePhi->getOpcode() == VPInstruction::ResumePhi &&
- "Expected a ResumePhi");
+ assert(isa<VPPhi>(ResumePhi) && "Expected a phi");
IVEndValues[WideIVR] = ResumePhi->getOperand(0);
ScalarPhiIRI->addOperand(ResumePhi);
continue;
@@ -9203,8 +9193,7 @@ static void addScalarResumePhis(VPRecipeBuilder &Builder, VPlan &Plan,
VPInstruction::ExtractLastElement, {ResumeFromVectorLoop}, {},
"vector.recur.extract");
StringRef Name = IsFOR ? "scalar.recur.init" : "bc.merge.rdx";
- auto *ResumePhiR = ScalarPHBuilder.createNaryOp(
- VPInstruction::ResumePhi,
+ auto *ResumePhiR = ScalarPHBuilder.createScalarPhi(
{ResumeFromVectorLoop, VectorPhiR->getStartValue()}, {}, Name);
ScalarPhiIRI->addOperand(ResumePhiR);
}
@@ -10406,9 +10395,7 @@ static void preparePlanForMainVectorLoop(VPlan &MainPlan, VPlan &EpiPlan) {
VPI->setOperand(1, Freeze);
if (UpdateResumePhis)
OrigStart->replaceUsesWithIf(Freeze, [Freeze](VPUser &U, unsigned) {
- return Freeze != &U && isa<VPInstruction>(&U) &&
- cast<VPInstruction>(&U)->getOpcode() ==
- VPInstruction::ResumePhi;
+ return Freeze != &U && isa<VPPhi>(&U);
});
}
};
@@ -10421,13 +10408,12 @@ static void preparePlanForMainVectorLoop(VPlan &MainPlan, VPlan &EpiPlan) {
// scalar (which will become vector) epilogue loop we are done. Otherwise
// create it below.
if (any_of(*MainScalarPH, [VectorTC](VPRecipeBase &R) {
- return match(&R, m_VPInstruction<VPInstruction::ResumePhi>(
- m_Specific(VectorTC), m_SpecificInt(0)));
+ return match(&R, m_VPInstruction<Instruction::PHI>(m_Specific(VectorTC),
+ m_SpecificInt(0)));
}))
return;
VPBuilder ScalarPHBuilder(MainScalarPH, MainScalarPH->begin());
- ScalarPHBuilder.createNaryOp(
- VPInstruction::ResumePhi,
+ ScalarPHBuilder.createScalarPhi(
{VectorTC, MainPlan.getCanonicalIV()->getStartValue()}, {},
"vec.epilog.resume.val");
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index e634de1e17c69..ebbffef2a4afe 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -887,11 +887,6 @@ class VPInstruction : public VPRecipeWithIRFlags,
SLPStore,
ActiveLaneMask,
ExplicitVectorLength,
- /// Creates a scalar phi in a leaf VPBB with a single predecessor in VPlan.
- /// The first operand is the incoming value from the predecessor in VPlan,
- /// the second operand is the incoming value for all other predecessors
- /// (which are currently not modeled in VPlan).
- ResumePhi,
CalculateTripCountMinusVF,
// Increment the canonical IV separately for each unrolled part.
CanonicalIVIncrementForPart,
@@ -1160,6 +1155,8 @@ class VPPhiAccessors {
return getAsRecipe()->getNumOperands();
}
+ void removeIncomingValue(VPBlockBase *VPB) const;
+
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the recipe.
void printPhiOperands(raw_ostream &O, VPSlotTracker &SlotTracker) const;
@@ -1170,11 +1167,15 @@ struct VPPhi : public VPInstruction, public VPPhiAccessors {
VPPhi(ArrayRef<VPValue *> Operands, DebugLoc DL, const Twine &Name = "")
: VPInstruction(Instruction::PHI, Operands, DL, Name) {}
- static inline bool classof(const VPRecipeBase *U) {
+ static inline bool classof(const VPUser *U) {
auto *R = dyn_cast<VPInstruction>(U);
return R && R->getOpcode() == Instruction::PHI;
}
+ VPPhi *clone() override {
+ return new VPPhi(operands(), getDebugLoc(), getName());
+ }
+
void execute(VPTransformState &State) override;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -3737,6 +3738,15 @@ VPPhiAccessors::getIncomingBlock(unsigned Idx) const {
return getAsRecipe()->getParent()->getCFGPredecessor(Idx);
}
+inline void VPPhiAccessors::removeIncomingValue(VPBlockBase *VPB) const {
+ VPRecipeBase *R = const_cast<VPRecipeBase *>(getAsRecipe());
+ const VPBasicBlock *Parent = R->getParent();
+ assert(R->getNumOperands() == Parent->getNumPredecessors());
+ auto I = find(Parent->getPredecessors(), VPB);
+ R->getOperand(I - Parent->getPredecessors().begin())->removeUser(*R);
+ R->removeOperand(I - Parent->getPredecessors().begin());
+}
+
/// A special type of VPBasicBlock that wraps an existing IR basic block.
/// Recipes of the block get added before the first non-phi instruction in the
/// wrapped block.
@@ -4246,7 +4256,8 @@ class VPlan {
/// that this relies on unneeded branches to the scalar tail loop being
/// removed.
bool hasScalarTail() const {
- return getScalarPreheader()->getNumPredecessors() != 0;
+ return getScalarPreheader()->getNumPredecessors() != 0 &&
+ getScalarPreheader()->getSinglePredecessor() != getEntry();
}
};
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index ac0f30cb4693c..926490bfad7d0 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -101,7 +101,6 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
return inferScalarType(R->getOperand(0));
case VPInstruction::FirstOrderRecurrenceSplice:
case VPInstruction::Not:
- case VPInstruction::ResumePhi:
case VPInstruction::CalculateTripCountMinusVF:
case VPInstruction::CanonicalIVIncrementForPart:
case VPInstruction::AnyOf:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 287bc93ce496a..204af8f12311b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -507,8 +507,12 @@ void VPlanTransforms::prepareForVectorization(
cast<VPBasicBlock>(HeaderVPB),
cast<VPBasicBlock>(LatchVPB), Range);
HandledUncountableEarlyExit = true;
+ } else {
+ for (VPRecipeBase &R : cast<VPIRBasicBlock>(EB)->phis()) {
+ if (auto *PhiR = dyn_cast<VPIRPhi>(&R))
+ PhiR->removeIncomingValue(Pred);
+ }
}
-
cast<VPBasicBlock>(Pred)->getTerminator()->eraseFromParent();
VPBlockUtils::disconnectBlocks(Pred, EB);
}
@@ -541,23 +545,13 @@ void VPlanTransforms::prepareForVectorization(
// Thus if tail is to be folded, we know we don't need to run the
// remainder and we can set the condition to true.
// 3) Otherwise, construct a runtime check.
+ VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
+ // Also connect the entry block to the scalar preheader.
+ VPBlockUtils::connectBlocks(Plan.getEntry(), ScalarPH);
+ Plan.getEntry()->swapSuccessors();
- if (!RequiresScalarEpilogueCheck) {
- if (auto *LatchExitVPB = MiddleVPBB->getSingleSuccessor())
- VPBlockUtils::disconnectBlocks(MiddleVPBB, LatchExitVPB);
- VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
- // The exit blocks are unreachable, remove their recipes to make sure no
- // users remain that may pessimize transforms.
- for (auto *EB : Plan.getExitBlocks()) {
- for (VPRecipeBase &R : make_early_inc_range(*EB))
- R.eraseFromParent();
- }
+ if (MiddleVPBB->getNumSuccessors() != 2)
return;
- }
-
- // The connection order corresponds to the operands of the conditional branch,
- // with the middle block already connected to the exit block.
- VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
auto *ScalarLatchTerm = TheLoop->getLoopLatch()->getTerminator();
// Here we use the same DebugLoc as the scalar loop latch terminator instead
@@ -565,13 +559,17 @@ void VPlanTransforms::prepareForVectorization(
// different line numbers and we want to avoid awkward line stepping while
// debugging. Eg. if the compare has got a line number inside the loop.
VPBuilder Builder(MiddleVPBB);
- VPValue *Cmp =
- TailFolded
- ? Plan.getOrAddLiveIn(ConstantInt::getTrue(
- IntegerType::getInt1Ty(TripCount->getType()->getContext())))
- : Builder.createICmp(CmpInst::ICMP_EQ, Plan.getTripCount(),
- &Plan.getVectorTripCount(),
- ScalarLatchTerm->getDebugLoc(), "cmp.n");
+ VPValue *Cmp;
+ if (TailFolded)
+ Cmp = Plan.getOrAddLiveIn(ConstantInt::getTrue(
+ IntegerType::getInt1Ty(TripCount->getType()->getContext())));
+ else if (!RequiresScalarEpilogueCheck)
+ Cmp = Plan.getOrAddLiveIn(ConstantInt::getFalse(
+ IntegerType::getInt1Ty(TripCount->getType()->getContext())));
+ else
+ Cmp = Builder.createICmp(CmpInst::ICMP_EQ, Plan.getTripCount(),
+ &Plan.getVectorTripCount(),
+ ScalarLatchTerm->getDebugLoc(), "cmp.n");
Builder.createNaryOp(VPInstruction::BranchOnCond, {Cmp},
ScalarLatchTerm->getDebugLoc());
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 3fa6a21b80b17..f4beb5b6d11e3 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -754,17 +754,6 @@ Value *VPInstruction::generate(VPTransformState &State) {
Value *Addend = State.get(getOperand(1), VPLane(0));
return Builder.CreatePtrAdd(Ptr, Addend, Name, getGEPNoWrapFlags());
}
- case VPInstruction::ResumePhi: {
- auto *NewPhi =
- Builder.CreatePHI(State.TypeAnalysis.inferScalarType(this), 2, Name);
- for (const auto &[IncVPV, PredVPBB] :
- zip(operands(), getParent()->getPredecessors())) {
- Value *IncV = State.get(IncVPV, /* IsScalar */ true);
- BasicBlock *PredBB = State.CFG.VPBB2IRBB.at(cast<VPBasicBlock>(PredVPBB));
- NewPhi->addIncoming(IncV, PredBB);
- }
- return NewPhi;
- }
case VPInstruction::AnyOf: {
Value *A = State.get(getOperand(0));
return Builder.CreateOrReduce(A);
@@ -863,8 +852,7 @@ bool VPInstruction::isVectorToScalar() const {
}
bool VPInstruction::isSingleScalar() const {
- return getOpcode() == VPInstruction::ResumePhi ||
- getOpcode() == Instruction::PHI;
+ return getOpcode() == Instruction::PHI;
}
#if !defined(NDEBUG)
@@ -963,7 +951,6 @@ bool VPInstruction::onlyFirstLaneUsed(const VPValue *Op) const {
case VPInstruction::CanonicalIVIncrementForPart:
case VPInstruction::BranchOnCount:
case VPInstruction::BranchOnCond:
- case VPInstruction::ResumePhi:
return true;
case VPInstruction::PtrAdd:
return Op == getOperand(0) || vputils::onlyFirstLaneUsed(this);
@@ -1020,9 +1007,6 @@ void VPInstruction::print(raw_ostream &O, const Twine &Indent,
case VPInstruction::ActiveLaneMask:
O << "active lane mask";
break;
- case VPInstruction::ResumePhi:
- O << "resume-phi";
- break;
case VPInstruction::ExplicitVectorLength:
O << "EXPLICIT-VECTOR-LENGTH";
break;
@@ -1130,15 +1114,16 @@ void VPInstructionWithType::print(raw_ostream &O, const Twine &Indent,
void VPPhi::execute(VPTransformState &State) {
State.setDebugLocFrom(getDebugLoc());
- assert(getParent() ==
- getParent()->getPlan()->getVectorLoopRegion()->getEntry() &&
- "VPInstructions with PHI opcodes must be used for header phis only "
- "at the moment");
- BasicBlock *VectorPH = State.CFG.VPBB2IRBB.at(getIncomingBlock(0));
- Value *Start = State.get(getIncomingValue(0), VPLane(0));
- PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, getName());
- Phi->addIncoming(Start, VectorPH);
- State.set(this, Phi, VPLane(0));
+ PHINode *NewPhi = State.Builder.CreatePHI(
+ State.TypeAnalysis.inferScalarType(this), 2, getName());
+ unsigned NumIncoming =
+ getParent()->getNumPredecessors() == 0 ? 1 : getNumIncoming();
+ for (unsigned Idx = 0; Idx != NumIncoming; ++Idx) {
+ Value *IncV = State.get(getIncomingValue(Idx), /* IsScalar */ true);
+ BasicBlock *PredBB = State.CFG.VPBB2IRBB.at(getIncomingBlock(Idx));
+ NewPhi->addIncoming(IncV, PredBB);
+ }
+ State.set(this, NewPhi, VPLane(0));
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index f1c466e3208be..e1c0e505d7c90 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1732,11 +1732,21 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
using namespace llvm::VPlanPatternMatch;
for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
vp_depth_first_shallow(Plan.getEntry()))) {
- if (VPBB->getNumSuccessors() != 2 ||
- !match(&VPBB->back(), m_BranchOnCond(m_True())))
+ VPValue *Cond;
+ if (VPBB->getNumSuccessors() != 2 || VPBB->empty() ||
+ !match(&VPBB->back(), m_BranchOnCond(m_VPValue(Cond))))
continue;
- VPBasicBlock *RemovedSucc = cast<VPBasicBlock>(VPBB->getSuccessors()[1]);
+ unsigned RemovedIdx;
+ if (match(Cond, m_True()))
+ RemovedIdx = 1;
+ else if (match(Cond, m_False()))
+ RemovedIdx = 0;
+ else
+ continue;
+
+ VPBasicBlock *RemovedSucc =
+ cast<VPBasicBlock>(VPBB->getSuccessors()[RemovedIdx]);
const auto &Preds = RemovedSucc->getPredecessors();
assert(count(Preds, VPBB) == 1 &&
"There must be a single edge between VPBB and its successor");
@@ -1745,12 +1755,14 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
// Values coming from VPBB into ResumePhi recipes of RemoveSucc are removed
// from these recipes.
for (VPRecipeBase &R : make_early_inc_range(*RemovedSucc)) {
-...
[truncated]
|
|
@llvm/pr-subscribers-backend-risc-v Author: Florian Hahn (fhahn) ChangesSimplify branch on false, starting with the branch from the middle block Depends on #140405. Patch is 101.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140409.diff 32 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 148b187f4f25d..6c3a2ce941c54 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -490,7 +490,7 @@ class InnerLoopVectorizer {
MinProfitableTripCount(MinProfitableTripCount), UF(UnrollFactor),
Builder(PSE.getSE()->getContext()), Cost(CM), BFI(BFI), PSI(PSI),
RTChecks(RTChecks), Plan(Plan),
- VectorPHVPB(Plan.getEntry()->getSingleSuccessor()) {}
+ VectorPHVPB(Plan.getEntry()->getSuccessors()[1]) {}
virtual ~InnerLoopVectorizer() = default;
@@ -2365,22 +2365,19 @@ InnerLoopVectorizer::getOrCreateVectorTripCount(BasicBlock *InsertBlock) {
void InnerLoopVectorizer::introduceCheckBlockInVPlan(BasicBlock *CheckIRBB) {
VPBlockBase *ScalarPH = Plan.getScalarPreheader();
VPBlockBase *PreVectorPH = VectorPHVPB->getSinglePredecessor();
- if (PreVectorPH->getNumSuccessors() != 1) {
- assert(PreVectorPH->getNumSuccessors() == 2 && "Expected 2 successors");
- assert(PreVectorPH->getSuccessors()[0] == ScalarPH &&
- "Unexpected successor");
- VPIRBasicBlock *CheckVPIRBB = Plan.createVPIRBasicBlock(CheckIRBB);
- VPBlockUtils::insertOnEdge(PreVectorPH, VectorPHVPB, CheckVPIRBB);
- PreVectorPH = CheckVPIRBB;
- }
+ assert(PreVectorPH->getNumSuccessors() == 2 && "Expected 2 successors");
+ assert(PreVectorPH->getSuccessors()[0] == ScalarPH && "Unexpected successor");
+ VPIRBasicBlock *CheckVPIRBB = Plan.createVPIRBasicBlock(CheckIRBB);
+ VPBlockUtils::insertOnEdge(PreVectorPH, VectorPHVPB, CheckVPIRBB);
+ PreVectorPH = CheckVPIRBB;
VPBlockUtils::connectBlocks(PreVectorPH, ScalarPH);
PreVectorPH->swapSuccessors();
// We just connected a new block to the scalar preheader. Update all
// ResumePhis by adding an incoming value for it, replicating the last value.
for (VPRecipeBase &R : *cast<VPBasicBlock>(ScalarPH)) {
- auto *ResumePhi = dyn_cast<VPInstruction>(&R);
- if (!ResumePhi || ResumePhi->getOpcode() != VPInstruction::ResumePhi)
+ auto *ResumePhi = cast<VPPhi>(&R);
+ if (ResumePhi->getNumIncoming() == ScalarPH->getNumPredecessors())
continue;
ResumePhi->addOperand(
ResumePhi->getOperand(ResumePhi->getNumOperands() - 1));
@@ -2463,9 +2460,6 @@ void InnerLoopVectorizer::emitIterationCountCheck(BasicBlock *Bypass) {
setBranchWeights(BI, MinItersBypassWeights, /*IsExpected=*/false);
ReplaceInstWithInst(TCCheckBlock->getTerminator(), &BI);
LoopBypassBlocks.push_back(TCCheckBlock);
-
- // TODO: Wrap LoopVectorPreHeader in VPIRBasicBlock here.
- introduceCheckBlockInVPlan(TCCheckBlock);
}
BasicBlock *InnerLoopVectorizer::emitSCEVChecks(BasicBlock *Bypass) {
@@ -2530,7 +2524,8 @@ BasicBlock *InnerLoopVectorizer::emitMemRuntimeChecks(BasicBlock *Bypass) {
static void replaceVPBBWithIRVPBB(VPBasicBlock *VPBB, BasicBlock *IRBB) {
VPIRBasicBlock *IRVPBB = VPBB->getPlan()->createVPIRBasicBlock(IRBB);
for (auto &R : make_early_inc_range(*VPBB)) {
- assert(!R.isPhi() && "Tried to move phi recipe to end of block");
+ assert((IRVPBB->empty() || IRVPBB->back().isPhi() || !R.isPhi()) &&
+ "Tried to move phi recipe to end of block");
R.moveBefore(*IRVPBB, IRVPBB->end());
}
@@ -7768,10 +7763,7 @@ static void fixReductionScalarResumeWhenVectorizingEpilog(
// created a bc.merge.rdx Phi after the main vector body. Ensure that we carry
// over the incoming values correctly.
using namespace VPlanPatternMatch;
- auto IsResumePhi = [](VPUser *U) {
- auto *VPI = dyn_cast<VPInstruction>(U);
- return VPI && VPI->getOpcode() == VPInstruction::ResumePhi;
- };
+ auto IsResumePhi = [](VPUser *U) { return isa<VPPhi>(U); };
assert(count_if(EpiRedResult->users(), IsResumePhi) == 1 &&
"ResumePhi must have a single user");
auto *EpiResumePhiVPI =
@@ -7837,7 +7829,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
// 1. Set up the skeleton for vectorization, including vector pre-header and
// middle block. The vector loop is created during VPlan execution.
- VPBasicBlock *VectorPH = cast<VPBasicBlock>(Entry->getSingleSuccessor());
+ VPBasicBlock *VectorPH = cast<VPBasicBlock>(Entry->getSuccessors()[1]);
State.CFG.PrevBB = ILV.createVectorizedLoopSkeleton();
if (VectorizingEpilogue)
VPlanTransforms::removeDeadRecipes(BestVPlan);
@@ -8070,7 +8062,8 @@ EpilogueVectorizerMainLoop::emitIterationCountCheck(BasicBlock *Bypass,
setBranchWeights(BI, MinItersBypassWeights, /*IsExpected=*/false);
ReplaceInstWithInst(TCCheckBlock->getTerminator(), &BI);
- introduceCheckBlockInVPlan(TCCheckBlock);
+ if (!ForEpilogue)
+ introduceCheckBlockInVPlan(TCCheckBlock);
return TCCheckBlock;
}
@@ -8200,7 +8193,6 @@ EpilogueVectorizerEpilogueLoop::emitMinimumVectorEpilogueIterCountCheck(
Plan.setEntry(NewEntry);
// OldEntry is now dead and will be cleaned up when the plan gets destroyed.
- introduceCheckBlockInVPlan(Insert);
return Insert;
}
@@ -9146,9 +9138,8 @@ static VPInstruction *addResumePhiRecipeForInduction(
WideIV->getDebugLoc());
}
- auto *ResumePhiRecipe =
- ScalarPHBuilder.createNaryOp(VPInstruction::ResumePhi, {EndValue, Start},
- WideIV->getDebugLoc(), "bc.resume.val");
+ auto *ResumePhiRecipe = ScalarPHBuilder.createScalarPhi(
+ {EndValue, Start}, WideIV->getDebugLoc(), "bc.resume.val");
return ResumePhiRecipe;
}
@@ -9160,7 +9151,7 @@ static void addScalarResumePhis(VPRecipeBuilder &Builder, VPlan &Plan,
DenseMap<VPValue *, VPValue *> &IVEndValues) {
VPTypeAnalysis TypeInfo(Plan.getCanonicalIV()->getScalarType());
auto *ScalarPH = Plan.getScalarPreheader();
- auto *MiddleVPBB = cast<VPBasicBlock>(ScalarPH->getSinglePredecessor());
+ auto *MiddleVPBB = cast<VPBasicBlock>(ScalarPH->getPredecessors()[0]);
VPRegionBlock *VectorRegion = Plan.getVectorLoopRegion();
VPBuilder VectorPHBuilder(
cast<VPBasicBlock>(VectorRegion->getSinglePredecessor()));
@@ -9177,8 +9168,7 @@ static void addScalarResumePhis(VPRecipeBuilder &Builder, VPlan &Plan,
if (VPInstruction *ResumePhi = addResumePhiRecipeForInduction(
WideIVR, VectorPHBuilder, ScalarPHBuilder, TypeInfo,
&Plan.getVectorTripCount())) {
- assert(ResumePhi->getOpcode() == VPInstruction::ResumePhi &&
- "Expected a ResumePhi");
+ assert(isa<VPPhi>(ResumePhi) && "Expected a phi");
IVEndValues[WideIVR] = ResumePhi->getOperand(0);
ScalarPhiIRI->addOperand(ResumePhi);
continue;
@@ -9203,8 +9193,7 @@ static void addScalarResumePhis(VPRecipeBuilder &Builder, VPlan &Plan,
VPInstruction::ExtractLastElement, {ResumeFromVectorLoop}, {},
"vector.recur.extract");
StringRef Name = IsFOR ? "scalar.recur.init" : "bc.merge.rdx";
- auto *ResumePhiR = ScalarPHBuilder.createNaryOp(
- VPInstruction::ResumePhi,
+ auto *ResumePhiR = ScalarPHBuilder.createScalarPhi(
{ResumeFromVectorLoop, VectorPhiR->getStartValue()}, {}, Name);
ScalarPhiIRI->addOperand(ResumePhiR);
}
@@ -10406,9 +10395,7 @@ static void preparePlanForMainVectorLoop(VPlan &MainPlan, VPlan &EpiPlan) {
VPI->setOperand(1, Freeze);
if (UpdateResumePhis)
OrigStart->replaceUsesWithIf(Freeze, [Freeze](VPUser &U, unsigned) {
- return Freeze != &U && isa<VPInstruction>(&U) &&
- cast<VPInstruction>(&U)->getOpcode() ==
- VPInstruction::ResumePhi;
+ return Freeze != &U && isa<VPPhi>(&U);
});
}
};
@@ -10421,13 +10408,12 @@ static void preparePlanForMainVectorLoop(VPlan &MainPlan, VPlan &EpiPlan) {
// scalar (which will become vector) epilogue loop we are done. Otherwise
// create it below.
if (any_of(*MainScalarPH, [VectorTC](VPRecipeBase &R) {
- return match(&R, m_VPInstruction<VPInstruction::ResumePhi>(
- m_Specific(VectorTC), m_SpecificInt(0)));
+ return match(&R, m_VPInstruction<Instruction::PHI>(m_Specific(VectorTC),
+ m_SpecificInt(0)));
}))
return;
VPBuilder ScalarPHBuilder(MainScalarPH, MainScalarPH->begin());
- ScalarPHBuilder.createNaryOp(
- VPInstruction::ResumePhi,
+ ScalarPHBuilder.createScalarPhi(
{VectorTC, MainPlan.getCanonicalIV()->getStartValue()}, {},
"vec.epilog.resume.val");
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index e634de1e17c69..ebbffef2a4afe 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -887,11 +887,6 @@ class VPInstruction : public VPRecipeWithIRFlags,
SLPStore,
ActiveLaneMask,
ExplicitVectorLength,
- /// Creates a scalar phi in a leaf VPBB with a single predecessor in VPlan.
- /// The first operand is the incoming value from the predecessor in VPlan,
- /// the second operand is the incoming value for all other predecessors
- /// (which are currently not modeled in VPlan).
- ResumePhi,
CalculateTripCountMinusVF,
// Increment the canonical IV separately for each unrolled part.
CanonicalIVIncrementForPart,
@@ -1160,6 +1155,8 @@ class VPPhiAccessors {
return getAsRecipe()->getNumOperands();
}
+ void removeIncomingValue(VPBlockBase *VPB) const;
+
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the recipe.
void printPhiOperands(raw_ostream &O, VPSlotTracker &SlotTracker) const;
@@ -1170,11 +1167,15 @@ struct VPPhi : public VPInstruction, public VPPhiAccessors {
VPPhi(ArrayRef<VPValue *> Operands, DebugLoc DL, const Twine &Name = "")
: VPInstruction(Instruction::PHI, Operands, DL, Name) {}
- static inline bool classof(const VPRecipeBase *U) {
+ static inline bool classof(const VPUser *U) {
auto *R = dyn_cast<VPInstruction>(U);
return R && R->getOpcode() == Instruction::PHI;
}
+ VPPhi *clone() override {
+ return new VPPhi(operands(), getDebugLoc(), getName());
+ }
+
void execute(VPTransformState &State) override;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -3737,6 +3738,15 @@ VPPhiAccessors::getIncomingBlock(unsigned Idx) const {
return getAsRecipe()->getParent()->getCFGPredecessor(Idx);
}
+inline void VPPhiAccessors::removeIncomingValue(VPBlockBase *VPB) const {
+ VPRecipeBase *R = const_cast<VPRecipeBase *>(getAsRecipe());
+ const VPBasicBlock *Parent = R->getParent();
+ assert(R->getNumOperands() == Parent->getNumPredecessors());
+ auto I = find(Parent->getPredecessors(), VPB);
+ R->getOperand(I - Parent->getPredecessors().begin())->removeUser(*R);
+ R->removeOperand(I - Parent->getPredecessors().begin());
+}
+
/// A special type of VPBasicBlock that wraps an existing IR basic block.
/// Recipes of the block get added before the first non-phi instruction in the
/// wrapped block.
@@ -4246,7 +4256,8 @@ class VPlan {
/// that this relies on unneeded branches to the scalar tail loop being
/// removed.
bool hasScalarTail() const {
- return getScalarPreheader()->getNumPredecessors() != 0;
+ return getScalarPreheader()->getNumPredecessors() != 0 &&
+ getScalarPreheader()->getSinglePredecessor() != getEntry();
}
};
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index ac0f30cb4693c..926490bfad7d0 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -101,7 +101,6 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
return inferScalarType(R->getOperand(0));
case VPInstruction::FirstOrderRecurrenceSplice:
case VPInstruction::Not:
- case VPInstruction::ResumePhi:
case VPInstruction::CalculateTripCountMinusVF:
case VPInstruction::CanonicalIVIncrementForPart:
case VPInstruction::AnyOf:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 287bc93ce496a..204af8f12311b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -507,8 +507,12 @@ void VPlanTransforms::prepareForVectorization(
cast<VPBasicBlock>(HeaderVPB),
cast<VPBasicBlock>(LatchVPB), Range);
HandledUncountableEarlyExit = true;
+ } else {
+ for (VPRecipeBase &R : cast<VPIRBasicBlock>(EB)->phis()) {
+ if (auto *PhiR = dyn_cast<VPIRPhi>(&R))
+ PhiR->removeIncomingValue(Pred);
+ }
}
-
cast<VPBasicBlock>(Pred)->getTerminator()->eraseFromParent();
VPBlockUtils::disconnectBlocks(Pred, EB);
}
@@ -541,23 +545,13 @@ void VPlanTransforms::prepareForVectorization(
// Thus if tail is to be folded, we know we don't need to run the
// remainder and we can set the condition to true.
// 3) Otherwise, construct a runtime check.
+ VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
+ // Also connect the entry block to the scalar preheader.
+ VPBlockUtils::connectBlocks(Plan.getEntry(), ScalarPH);
+ Plan.getEntry()->swapSuccessors();
- if (!RequiresScalarEpilogueCheck) {
- if (auto *LatchExitVPB = MiddleVPBB->getSingleSuccessor())
- VPBlockUtils::disconnectBlocks(MiddleVPBB, LatchExitVPB);
- VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
- // The exit blocks are unreachable, remove their recipes to make sure no
- // users remain that may pessimize transforms.
- for (auto *EB : Plan.getExitBlocks()) {
- for (VPRecipeBase &R : make_early_inc_range(*EB))
- R.eraseFromParent();
- }
+ if (MiddleVPBB->getNumSuccessors() != 2)
return;
- }
-
- // The connection order corresponds to the operands of the conditional branch,
- // with the middle block already connected to the exit block.
- VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
auto *ScalarLatchTerm = TheLoop->getLoopLatch()->getTerminator();
// Here we use the same DebugLoc as the scalar loop latch terminator instead
@@ -565,13 +559,17 @@ void VPlanTransforms::prepareForVectorization(
// different line numbers and we want to avoid awkward line stepping while
// debugging. Eg. if the compare has got a line number inside the loop.
VPBuilder Builder(MiddleVPBB);
- VPValue *Cmp =
- TailFolded
- ? Plan.getOrAddLiveIn(ConstantInt::getTrue(
- IntegerType::getInt1Ty(TripCount->getType()->getContext())))
- : Builder.createICmp(CmpInst::ICMP_EQ, Plan.getTripCount(),
- &Plan.getVectorTripCount(),
- ScalarLatchTerm->getDebugLoc(), "cmp.n");
+ VPValue *Cmp;
+ if (TailFolded)
+ Cmp = Plan.getOrAddLiveIn(ConstantInt::getTrue(
+ IntegerType::getInt1Ty(TripCount->getType()->getContext())));
+ else if (!RequiresScalarEpilogueCheck)
+ Cmp = Plan.getOrAddLiveIn(ConstantInt::getFalse(
+ IntegerType::getInt1Ty(TripCount->getType()->getContext())));
+ else
+ Cmp = Builder.createICmp(CmpInst::ICMP_EQ, Plan.getTripCount(),
+ &Plan.getVectorTripCount(),
+ ScalarLatchTerm->getDebugLoc(), "cmp.n");
Builder.createNaryOp(VPInstruction::BranchOnCond, {Cmp},
ScalarLatchTerm->getDebugLoc());
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 3fa6a21b80b17..f4beb5b6d11e3 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -754,17 +754,6 @@ Value *VPInstruction::generate(VPTransformState &State) {
Value *Addend = State.get(getOperand(1), VPLane(0));
return Builder.CreatePtrAdd(Ptr, Addend, Name, getGEPNoWrapFlags());
}
- case VPInstruction::ResumePhi: {
- auto *NewPhi =
- Builder.CreatePHI(State.TypeAnalysis.inferScalarType(this), 2, Name);
- for (const auto &[IncVPV, PredVPBB] :
- zip(operands(), getParent()->getPredecessors())) {
- Value *IncV = State.get(IncVPV, /* IsScalar */ true);
- BasicBlock *PredBB = State.CFG.VPBB2IRBB.at(cast<VPBasicBlock>(PredVPBB));
- NewPhi->addIncoming(IncV, PredBB);
- }
- return NewPhi;
- }
case VPInstruction::AnyOf: {
Value *A = State.get(getOperand(0));
return Builder.CreateOrReduce(A);
@@ -863,8 +852,7 @@ bool VPInstruction::isVectorToScalar() const {
}
bool VPInstruction::isSingleScalar() const {
- return getOpcode() == VPInstruction::ResumePhi ||
- getOpcode() == Instruction::PHI;
+ return getOpcode() == Instruction::PHI;
}
#if !defined(NDEBUG)
@@ -963,7 +951,6 @@ bool VPInstruction::onlyFirstLaneUsed(const VPValue *Op) const {
case VPInstruction::CanonicalIVIncrementForPart:
case VPInstruction::BranchOnCount:
case VPInstruction::BranchOnCond:
- case VPInstruction::ResumePhi:
return true;
case VPInstruction::PtrAdd:
return Op == getOperand(0) || vputils::onlyFirstLaneUsed(this);
@@ -1020,9 +1007,6 @@ void VPInstruction::print(raw_ostream &O, const Twine &Indent,
case VPInstruction::ActiveLaneMask:
O << "active lane mask";
break;
- case VPInstruction::ResumePhi:
- O << "resume-phi";
- break;
case VPInstruction::ExplicitVectorLength:
O << "EXPLICIT-VECTOR-LENGTH";
break;
@@ -1130,15 +1114,16 @@ void VPInstructionWithType::print(raw_ostream &O, const Twine &Indent,
void VPPhi::execute(VPTransformState &State) {
State.setDebugLocFrom(getDebugLoc());
- assert(getParent() ==
- getParent()->getPlan()->getVectorLoopRegion()->getEntry() &&
- "VPInstructions with PHI opcodes must be used for header phis only "
- "at the moment");
- BasicBlock *VectorPH = State.CFG.VPBB2IRBB.at(getIncomingBlock(0));
- Value *Start = State.get(getIncomingValue(0), VPLane(0));
- PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, getName());
- Phi->addIncoming(Start, VectorPH);
- State.set(this, Phi, VPLane(0));
+ PHINode *NewPhi = State.Builder.CreatePHI(
+ State.TypeAnalysis.inferScalarType(this), 2, getName());
+ unsigned NumIncoming =
+ getParent()->getNumPredecessors() == 0 ? 1 : getNumIncoming();
+ for (unsigned Idx = 0; Idx != NumIncoming; ++Idx) {
+ Value *IncV = State.get(getIncomingValue(Idx), /* IsScalar */ true);
+ BasicBlock *PredBB = State.CFG.VPBB2IRBB.at(getIncomingBlock(Idx));
+ NewPhi->addIncoming(IncV, PredBB);
+ }
+ State.set(this, NewPhi, VPLane(0));
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index f1c466e3208be..e1c0e505d7c90 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1732,11 +1732,21 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
using namespace llvm::VPlanPatternMatch;
for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
vp_depth_first_shallow(Plan.getEntry()))) {
- if (VPBB->getNumSuccessors() != 2 ||
- !match(&VPBB->back(), m_BranchOnCond(m_True())))
+ VPValue *Cond;
+ if (VPBB->getNumSuccessors() != 2 || VPBB->empty() ||
+ !match(&VPBB->back(), m_BranchOnCond(m_VPValue(Cond))))
continue;
- VPBasicBlock *RemovedSucc = cast<VPBasicBlock>(VPBB->getSuccessors()[1]);
+ unsigned RemovedIdx;
+ if (match(Cond, m_True()))
+ RemovedIdx = 1;
+ else if (match(Cond, m_False()))
+ RemovedIdx = 0;
+ else
+ continue;
+
+ VPBasicBlock *RemovedSucc =
+ cast<VPBasicBlock>(VPBB->getSuccessors()[RemovedIdx]);
const auto &Preds = RemovedSucc->getPredecessors();
assert(count(Preds, VPBB) == 1 &&
"There must be a single edge between VPBB and its successor");
@@ -1745,12 +1755,14 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
// Values coming from VPBB into ResumePhi recipes of RemoveSucc are removed
// from these recipes.
for (VPRecipeBase &R : make_early_inc_range(*RemovedSucc)) {
-...
[truncated]
|
fhahn
changed the title
Use regular VPPhi instead of a separate opcode for resume phis. This removes an unneeded specialized opcode and unifies the code (verification, printing, updating when CFG is changed). Depends on llvm#140132.
fhahn
changed the title
Simplify branch on false, starting with the branch from the middle block to the scalar preheader. Depends on llvm#140405.
fhahn
force-pushed
the
vplan-branch-on-false
branch
from
0fa741c to
69ad1de
Compare
ayalz
reviewed
May 29, 2025
| @@ -1160,6 +1155,8 @@ class VPPhiAccessors { | |||
| return getAsRecipe()->getNumOperands(); | |||
| } | |||
|
|
|||
| void removeIncomingValue(VPBlockBase *VPB) const; | |||
There was a problem hiding this comment.
Suggested change
| void removeIncomingValue(VPBlockBase *VPB) const; | |
| /// Removes the incoming value corresponding to \p IncomingBlock, which must be a predecessor. | |
| void removeIncomingValueFor(VPBlockBase *IncomingBlock) const; |
| inline void VPPhiAccessors::removeIncomingValue(VPBlockBase *VPB) const { | ||
| VPRecipeBase *R = const_cast<VPRecipeBase *>(getAsRecipe()); | ||
| const VPBasicBlock *Parent = R->getParent(); | ||
| assert(R->getNumOperands() == Parent->getNumPredecessors()); |
There was a problem hiding this comment.
Suggested change
| assert(R->getNumOperands() == Parent->getNumPredecessors()); | |
| assert(R->getNumOperands() == Parent->getNumPredecessors() && "Number of phi operands must match number of predecessors"); |
| VPRecipeBase *R = const_cast<VPRecipeBase *>(getAsRecipe()); | ||
| const VPBasicBlock *Parent = R->getParent(); | ||
| assert(R->getNumOperands() == Parent->getNumPredecessors()); | ||
| auto I = find(Parent->getPredecessors(), VPB); |
There was a problem hiding this comment.
nit: may be clearer to use distance as in
Suggested change
| auto I = find(Parent->getPredecessors(), VPB); | |
| auto &Preds = R->getParent()->getPredecessors(); | |
| assert(R->getNumOperands() == Preds.size() && "Number of phi operands must match number of predecessors"); | |
| unsigned Position = std::distance(Preds.begin(), find(Preds, IncomingBlock)); |
There was a problem hiding this comment.
Done. Also moved to VPlanRecipes.cpp
| @@ -507,8 +507,12 @@ void VPlanTransforms::prepareForVectorization( | |||
| cast<VPBasicBlock>(HeaderVPB), | |||
| cast<VPBasicBlock>(LatchVPB), Range); | |||
| HandledUncountableEarlyExit = true; | |||
| } else { | |||
| for (VPRecipeBase &R : cast<VPIRBasicBlock>(EB)->phis()) { | |||
There was a problem hiding this comment.
Is the cast needed given that EB is a VPIRBasicBlock?
There was a problem hiding this comment.
not needed, removed thanks
| @@ -507,8 +507,12 @@ void VPlanTransforms::prepareForVectorization( | |||
| cast<VPBasicBlock>(HeaderVPB), | |||
| cast<VPBasicBlock>(LatchVPB), Range); | |||
| HandledUncountableEarlyExit = true; | |||
| } else { | |||
| for (VPRecipeBase &R : cast<VPIRBasicBlock>(EB)->phis()) { | |||
| if (auto *PhiR = dyn_cast<VPIRPhi>(&R)) | |||
There was a problem hiding this comment.
Can R be anything but a VPIRPhi?
| @@ -199,8 +200,12 @@ raw_ostream &operator<<(raw_ostream &OS, const VPRecipeBase &R); | |||
| /// This class augments VPValue with operands which provide the inverse def-use | |||
| /// edges from VPValue's users to their defs. | |||
| class VPUser { | |||
| friend class VPPhiAccessors; | |||
There was a problem hiding this comment.
Why/is this befriending needed?
There was a problem hiding this comment.
removeOperand is added as private, to discourage its used unless really necessary, hence befriending. Could also expose as protected or public instead
There was a problem hiding this comment.
Worth clarifying this discouraging motivation for friend - removeOperand() could be done by RAUW a new similar recipe built w/o the prescribed operand.
Currently removeOperand() is needed only by removeIncomingValue() for VPIRPhi's only. Suffice to provide VPIRPhi::removeIncomingValueFor() which would access a protected removeOperand()?
| } else { | ||
| for (VPRecipeBase &R : cast<VPIRBasicBlock>(EB)->phis()) { | ||
| if (auto *PhiR = dyn_cast<VPIRPhi>(&R)) | ||
| PhiR->removeIncomingValue(Pred); |
There was a problem hiding this comment.
Is this part of disconnecting (countable) early exits? Can PhiR remain w/o any incoming value, and if so should it be removed?
There was a problem hiding this comment.
Yes this is related to disconnecting the early exit, complementing VPBlockUtils::disconnectBlocks(Pred, EB) below
|
|
||
| // The connection order corresponds to the operands of the conditional branch, | ||
| // with the middle block already connected to the exit block. | ||
| VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH); | ||
|
|
||
| auto *ScalarLatchTerm = TheLoop->getLoopLatch()->getTerminator(); | ||
| // Here we use the same DebugLoc as the scalar loop latch terminator instead | ||
| // of the corresponding compare because they may have ended up with | ||
| // different line numbers and we want to avoid awkward line stepping while | ||
| // debugging. Eg. if the compare has got a line number inside the loop. |
There was a problem hiding this comment.
Suggested change
| // debugging. Eg. if the compare has got a line number inside the loop. | |
| // debugging. E.g., if the compare has got a line number inside the loop. |
| auto *ResumePhi = dyn_cast<VPInstruction>(&R); | ||
| if (!ResumePhi || ResumePhi->getOpcode() != VPInstruction::ResumePhi) | ||
| continue; | ||
| // VPPhis by adding an incoming value for it, replicating the last value. |
There was a problem hiding this comment.
nit: take the number of ScalarPH predecessors once, here, reuse it repeatedly inside the loop, asserting it is one more than the number of ResumePhi's operands.
| auto *VPI = dyn_cast<VPInstruction>(&R); | ||
| if (!VPI || VPI->getOpcode() != VPInstruction::ResumePhi) | ||
| if (isa<VPIRPhi>(&R)) { | ||
| assert(RemovedSucc->getNumPredecessors() == 1); |
ayalz
reviewed
May 29, 2025
| @@ -1847,11 +1847,21 @@ static void removeBranchOnCondTrue(VPlan &Plan) { | |||
| using namespace llvm::VPlanPatternMatch; | |||
There was a problem hiding this comment.
Name of removeBranchOnCondTrue() should change, to, e.g., removeBranchOnConst()?
ayalz
reviewed
May 29, 2025
Comment on lines
+2384
to
+2390
| (void)NumPredecessors; | ||
| for (VPRecipeBase &R : cast<VPBasicBlock>(ScalarPH)->phis()) { | ||
| auto *ResumePhi = cast<VPPhi>(&R); | ||
| ResumePhi->addOperand( | ||
| ResumePhi->getOperand(ResumePhi->getNumOperands() - 1)); | ||
| assert(ResumePhi->getNumIncoming() == NumPredecessors && | ||
| "must have incoming values for all operands"); |
There was a problem hiding this comment.
Suggested change
| (void)NumPredecessors; | |
| for (VPRecipeBase &R : cast<VPBasicBlock>(ScalarPH)->phis()) { | |
| auto *ResumePhi = cast<VPPhi>(&R); | |
| ResumePhi->addOperand( | |
| ResumePhi->getOperand(ResumePhi->getNumOperands() - 1)); | |
| assert(ResumePhi->getNumIncoming() == NumPredecessors && | |
| "must have incoming values for all operands"); | |
| for (VPRecipeBase &R : cast<VPBasicBlock>(ScalarPH)->phis()) { | |
| assert(isa<VPPhi>(&R) && "Phi expected to be VPPhi"); | |
| assert(cast<VPPhi>(&R)->getNumIncoming() == NumPredecessors - 1 && | |
| "must have incoming values for all operands"); | |
| R->addOperand(R->getOperand(NumPredecessors - 1)); |
| @@ -1127,6 +1122,10 @@ class VPPhiAccessors { | |||
| return getAsRecipe()->getNumOperands(); | |||
| } | |||
|
|
|||
| /// Removes the incoming value corresponding to \p IncomingBlock, which must | |||
| /// be a predecessor. | |||
| void removeIncomingValue(VPBlockBase *IncomingBlock) const; | |||
There was a problem hiding this comment.
Suggested change
| void removeIncomingValue(VPBlockBase *IncomingBlock) const; | |
| void removeIncomingValueFor(VPBlockBase *IncomingBlock) const; |
?
| @@ -199,8 +200,12 @@ raw_ostream &operator<<(raw_ostream &OS, const VPRecipeBase &R); | |||
| /// This class augments VPValue with operands which provide the inverse def-use | |||
| /// edges from VPValue's users to their defs. | |||
| class VPUser { | |||
| friend class VPPhiAccessors; | |||
There was a problem hiding this comment.
Worth clarifying this discouraging motivation for friend - removeOperand() could be done by RAUW a new similar recipe built w/o the prescribed operand.
Currently removeOperand() is needed only by removeIncomingValue() for VPIRPhi's only. Suffice to provide VPIRPhi::removeIncomingValueFor() which would access a protected removeOperand()?
| using namespace llvm::VPlanPatternMatch; | ||
| for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>( | ||
| vp_depth_first_shallow(Plan.getEntry()))) { | ||
| VPValue *Cond; | ||
| if (VPBB->getNumSuccessors() != 2 || VPBB == Plan.getEntry() || |
There was a problem hiding this comment.
Independent: why is the entry block excluded?
Comment on lines
+1891
to
+1892
| VPI->replaceAllUsesWith( | ||
| B.createScalarPhi(NewOperands, VPI->getDebugLoc(), VPI->getName())); |
There was a problem hiding this comment.
This is the alternative to removeIncomingValueFor(VPBB), on a VPPhi?
|
|
||
| assert(MiddleVPBB->getNumSuccessors() == 2 && "must have 2 successors"); | ||
|
|
||
| // If needed, add a check in the middle block to see if we have completed |
There was a problem hiding this comment.
Suggested change
| // If needed, add a check in the middle block to see if we have completed | |
| // Add a check in the middle block to see if we have completed |
| // of the corresponding compare because they may have ended up with | ||
| // different line numbers and we want to avoid awkward line stepping while | ||
| // debugging. Eg. if the compare has got a line number inside the loop. | ||
| // If MiddleVPBB has a single successor the original loop exits via the latch |
There was a problem hiding this comment.
Suggested change
| // If MiddleVPBB has a single successor the original loop exits via the latch | |
| // If MiddleVPBB has a single successor then the original loop does not exit via the latch |
Comment on lines
-555
to
-556
| // The connection order corresponds to the operands of the conditional branch, | ||
| // with the middle block already connected to the exit block. |
There was a problem hiding this comment.
Above two lines should be discarded? Or updated to end with "if needed"?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Simplify branch on false, starting with the branch from the middle block
to the scalar preheader. Initially this helps simplifying the initial VPlan construction.
Depends on #140405.