[llvm] 10bd4cd - [VPlan] Remove ResumePhi opcode, use regular PHI instead (NFC). (#140405)
via llvm-commits
llvm-commits at lists.llvm.org
Fri May 30 04:50:11 PDT 2025
Author: Florian Hahn
Date: 2025-05-30T12:50:08+01:00
New Revision: 10bd4cd9cd0f11f158c0fe276441c65ba6bca30a
URL: https://github.com/llvm/llvm-project/commit/10bd4cd9cd0f11f158c0fe276441c65ba6bca30a
DIFF: https://github.com/llvm/llvm-project/commit/10bd4cd9cd0f11f158c0fe276441c65ba6bca30a.diff
LOG: [VPlan] Remove ResumePhi opcode, use regular PHI instead (NFC). (#140405)
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 https://github.com/llvm/llvm-project/pull/140132.
PR: https://github.com/llvm/llvm-project/pull/140405
Added:
Modified:
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/lib/Transforms/Vectorize/VPlan.h
llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 6954a5f2dab43..ecdba9d1a1032 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -2379,11 +2379,9 @@ void InnerLoopVectorizer::introduceCheckBlockInVPlan(BasicBlock *CheckIRBB) {
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)
- continue;
+ // VPPhis by adding an incoming value for it, replicating the last value.
+ for (VPRecipeBase &R : cast<VPBasicBlock>(ScalarPH)->phis()) {
+ auto *ResumePhi = cast<VPPhi>(&R);
ResumePhi->addOperand(
ResumePhi->getOperand(ResumePhi->getNumOperands() - 1));
}
@@ -2533,7 +2531,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 after a non-phi recipe");
R.moveBefore(*IRVPBB, IRVPBB->end());
}
@@ -7535,14 +7534,10 @@ 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;
- };
- assert(count_if(EpiRedResult->users(), IsResumePhi) == 1 &&
+ assert(count_if(EpiRedResult->users(), IsaPred<VPPhi>) == 1 &&
"ResumePhi must have a single user");
auto *EpiResumePhiVPI =
- cast<VPInstruction>(*find_if(EpiRedResult->users(), IsResumePhi));
+ cast<VPInstruction>(*find_if(EpiRedResult->users(), IsaPred<VPPhi>));
auto *EpiResumePhi = cast<PHINode>(State.get(EpiResumePhiVPI, true));
EpiResumePhi->setIncomingValueForBlock(
BypassBlock, MainResumePhi->getIncomingValueForBlock(BypassBlock));
@@ -8723,9 +8718,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;
}
@@ -8754,8 +8748,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;
@@ -8780,8 +8773,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);
}
@@ -9961,9 +9953,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);
});
}
};
@@ -9976,13 +9966,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 13c0d84f5e219..16c063c0a4a27 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,
@@ -1137,11 +1132,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)
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/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index d16c8b44891f5..5565c4b8aac1e 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -733,17 +733,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);
@@ -847,8 +836,7 @@ bool VPInstruction::isVectorToScalar() const {
}
bool VPInstruction::isSingleScalar() const {
- return getOpcode() == VPInstruction::ResumePhi ||
- getOpcode() == Instruction::PHI;
+ return getOpcode() == Instruction::PHI;
}
void VPInstruction::execute(VPTransformState &State) {
@@ -934,7 +922,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);
@@ -991,9 +978,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;
@@ -1101,11 +1085,20 @@ void VPInstructionWithType::print(raw_ostream &O, const Twine &Indent,
void VPPhi::execute(VPTransformState &State) {
State.setDebugLocFrom(getDebugLoc());
- 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 = getNumIncoming();
+ if (getParent() != getParent()->getPlan()->getScalarPreheader()) {
+ // TODO: Fixup all incoming values of header phis once recipes defining them
+ // are introduced.
+ NumIncoming = 1;
+ }
+ for (unsigned Idx = 0; Idx != NumIncoming; ++Idx) {
+ Value *IncV = State.get(getIncomingValue(Idx), VPLane(0));
+ 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)
@@ -1113,9 +1106,15 @@ void VPPhi::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
O << Indent << "EMIT" << (isSingleScalar() ? "-SCALAR" : "") << " ";
printAsOperand(O, SlotTracker);
- O << " = phi ";
-
- printPhiOperands(O, SlotTracker);
+ const VPBasicBlock *Parent = getParent();
+ if (Parent == Parent->getPlan()->getScalarPreheader()) {
+ // TODO: Use regular printing for resume-phis as well
+ O << " = resume-phi ";
+ printOperands(O, SlotTracker);
+ } else {
+ O << " = phi ";
+ printPhiOperands(O, SlotTracker);
+ }
}
#endif
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index b1dba1daed53d..beab52fc3b133 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1864,8 +1864,8 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
!isa<PHINode>(cast<VPIRInstruction>(&R)->getInstruction())) &&
!isa<VPHeaderPHIRecipe>(&R) &&
"Cannot update VPIRInstructions wrapping phis or header phis yet");
- auto *VPI = dyn_cast<VPInstruction>(&R);
- if (!VPI || VPI->getOpcode() != VPInstruction::ResumePhi)
+ auto *VPI = dyn_cast<VPPhi>(&R);
+ if (!VPI)
break;
VPBuilder B(VPI);
SmallVector<VPValue *> NewOperands;
@@ -1875,9 +1875,8 @@ static void removeBranchOnCondTrue(VPlan &Plan) {
continue;
NewOperands.push_back(Op);
}
- VPI->replaceAllUsesWith(B.createNaryOp(VPInstruction::ResumePhi,
- NewOperands, VPI->getDebugLoc(),
- VPI->getName()));
+ VPI->replaceAllUsesWith(
+ B.createScalarPhi(NewOperands, VPI->getDebugLoc(), VPI->getName()));
VPI->eraseFromParent();
}
// Disconnect blocks and remove the terminator. RemovedSucc will be deleted
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index 54cf8ac2ed04a..45010d0021581 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -243,9 +243,7 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
continue;
}
// TODO: Also verify VPPredInstPHIRecipe.
- if (isa<VPPredInstPHIRecipe>(UI) ||
- (isa<VPInstruction>(UI) && (cast<VPInstruction>(UI)->getOpcode() ==
- VPInstruction::ResumePhi)))
+ if (isa<VPPredInstPHIRecipe>(UI))
continue;
// If the user is in the same block, check it comes after R in the
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
index fa7a9aa5549ee..5394472381454 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
@@ -151,6 +151,7 @@ TEST_F(VPVerifierTest, VPPhiIncomingValueDoesntDominateIncomingBlock) {
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
+ VPBasicBlock *VPBB4 = Plan.createVPBasicBlock("");
VPInstruction *DefI = new VPInstruction(Instruction::Add, {Zero});
VPPhi *Phi = new VPPhi({DefI}, {});
@@ -162,6 +163,7 @@ TEST_F(VPVerifierTest, VPPhiIncomingValueDoesntDominateIncomingBlock) {
VPRegionBlock *R1 = Plan.createVPRegionBlock(VPBB3, VPBB3, "R1");
VPBlockUtils::connectBlocks(VPBB1, VPBB2);
VPBlockUtils::connectBlocks(VPBB2, R1);
+ VPBlockUtils::connectBlocks(VPBB4, Plan.getScalarHeader());
#if GTEST_HAS_STREAM_REDIRECTION
::testing::internal::CaptureStderr();
#endif
More information about the llvm-commits
mailing list