Skip to content

[VPlan] Add VPPhi subclass for VPInstruction with PHI opcodes.(NFC) #139151

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 3 commits into from
May 10, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[VPlan] Add new VPPhi subclass for VPInstruction with PHI opcodes.
Similarly to VPInstructionWithType and VPIRPhi, add VPPhi as a subclass
for VPInstruction. This allows implementing the VPPhiAccessors trait,
making available helpers for generic printing of incoming values /
blocks and accessors for incoming blocks and values.

It will also allow properly verifying def-uses for values used by
VPInstructions with PHI opcodes via
#124838.
  • Loading branch information
fhahn committed May 10, 2025
commit b354f580cbc9b067142fbf9d08f7a2d9563ed86a
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class VPBuilder {
VPInstruction *createScalarPhi(ArrayRef<VPValue *> IncomingValues,
DebugLoc DL, const Twine &Name = "") {
return tryInsertInstruction(
new VPInstruction(Instruction::PHI, IncomingValues, DL, Name));
new VPPhi(Instruction::PHI, IncomingValues, DL, Name));
}

/// Convert the input value \p Current to the corresponding value of an
Expand Down
22 changes: 22 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,28 @@ class VPPhiAccessors {
#endif
};

struct VPPhi : public VPInstruction, public VPPhiAccessors {
VPPhi(unsigned Opcode, ArrayRef<VPValue *> Operands, DebugLoc DL,
Copy link
Member

Choose a reason for hiding this comment

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

Can it have any opcode other than PHI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, dropped, thanks!

const Twine &Name = "")
: VPInstruction(Opcode, Operands, DL, Name) {}

static inline bool classof(const VPRecipeBase *U) {
auto *R = dyn_cast<VPInstruction>(U);
return R && R->getOpcode() == Instruction::PHI;
}

void execute(VPTransformState &State) override;

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the recipe.
void print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const override;
#endif

protected:
const VPRecipeBase *getAsRecipe() const override { return this; }
};

/// A recipe to wrap on original IR instruction not to be modified during
/// execution, except for PHIs. PHIs are modeled via the VPIRPhi subclass.
/// Expect PHIs, VPIRInstructions cannot have any operands.
Expand Down
35 changes: 25 additions & 10 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -492,16 +492,7 @@ Value *VPInstruction::generate(VPTransformState &State) {
return Builder.CreateCmp(getPredicate(), A, B, Name);
}
case Instruction::PHI: {
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(getParent()->getCFGPredecessor(0));
Value *Start = State.get(getOperand(0), VPLane(0));
PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, Name);
Phi->addIncoming(Start, VectorPH);
return Phi;
llvm_unreachable("should be handled by VPPhi::execute");
}
case Instruction::Select: {
bool OnlyFirstLaneUsed = vputils::onlyFirstLaneUsed(this);
Expand Down Expand Up @@ -1131,6 +1122,30 @@ void VPInstructionWithType::print(raw_ostream &O, const Twine &Indent,
}
#endif

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));
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void VPPhi::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
O << Indent << "EMIT ";
printAsOperand(O, SlotTracker);
O << " = phi ";

printPhiOperands(O, SlotTracker);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: worth separating this change of how the recipe is printed to a follow-up patch, associated with the test changes?
Patch should be marked NFC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I split this off from the patch and will land separately, thanks

}
#endif

VPIRInstruction *VPIRInstruction ::create(Instruction &I) {
if (auto *Phi = dyn_cast<PHINode>(&I))
return new VPIRPhi(*Phi);
Expand Down