Skip to content

[VPlan] Remove ResumePhi opcode, use regular PHI instead (NFC). #140405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 30, 2025
Merged
41 changes: 15 additions & 26 deletions llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
});
}
};
Expand All @@ -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");
}
Expand Down
11 changes: 5 additions & 6 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Comment on lines -890 to -893
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth retaining this documentation somewhere, possibly updated if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment wasn't accruate any longer; they now have incoming values for all predecssors, hence being 'full' phis.

ResumePhi,
CalculateTripCountMinusVF,
// Increment the canonical IV separately for each unrolled part.
CanonicalIVIncrementForPart,
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
49 changes: 24 additions & 25 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can retain how resume-phis are printed by checking if a phi is in the scalar preheader when printed in this patch, reducing its test diff and keeping it strictly NFC. Changing its printing can be done separately as a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks

break;
case VPInstruction::ExplicitVectorLength:
O << "EXPLICIT-VECTOR-LENGTH";
break;
Expand Down Expand Up @@ -1101,21 +1085,36 @@ 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)
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

Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}, {});
Expand All @@ -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
Expand Down
Loading