Skip to content

Commit

Permalink
[VPlan] Ensure start value of phis is the first op at construction (NFC)
Browse files Browse the repository at this point in the history
Header phi recipes have the start value (incoming from outside the loop)
as first operand. This wasn't the case for VPWidenPHIRecipes. Instead
the start value was picked during execute() by doing extra work.

To be in line with other recipes, ensure the operand order is as
expected during construction.
  • Loading branch information
fhahn committed Sep 22, 2023
1 parent 98eb28b commit d9f8316
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
20 changes: 20 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ void PlainCFGBuilder::setVPBBPredsFromBB(VPBasicBlock *VPBB, BasicBlock *BB) {
VPBB->setPredecessors(VPBBPreds);
}

static bool isHeaderBB(BasicBlock *BB, Loop *L) {
return L && BB == L->getHeader();
}

// Add operands to VPInstructions representing phi nodes from the input IR.
void PlainCFGBuilder::fixPhiNodes() {
for (auto *Phi : PhisToFix) {
Expand All @@ -100,6 +104,22 @@ void PlainCFGBuilder::fixPhiNodes() {
assert(VPPhi->getNumOperands() == 0 &&
"Expected VPInstruction with no operands.");

Loop *L = LI->getLoopFor(Phi->getParent());
if (isHeaderBB(Phi->getParent(), L)) {
// For header phis, make sure the incoming value from the loop
// predecessor is the first operand of the recipe.
assert(Phi->getNumOperands() == 2);
BasicBlock *LoopPred = L->getLoopPredecessor();
VPPhi->addIncoming(
getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopPred)),
BB2VPBB[LoopPred]);
BasicBlock *LoopLatch = L->getLoopLatch();
VPPhi->addIncoming(
getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopLatch)),
BB2VPBB[LoopLatch]);
continue;
}

for (unsigned I = 0; I != Phi->getNumOperands(); ++I)
VPPhi->addIncoming(getOrCreateVPOperand(Phi->getIncomingValue(I)),
BB2VPBB[Phi->getIncomingBlock(I)]);
Expand Down
18 changes: 1 addition & 17 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1626,23 +1626,7 @@ void VPWidenPHIRecipe::execute(VPTransformState &State) {
assert(EnableVPlanNativePath &&
"Non-native vplans are not expected to have VPWidenPHIRecipes.");

// Currently we enter here in the VPlan-native path for non-induction
// PHIs where all control flow is uniform. We simply widen these PHIs.
// Create a vector phi with no operands - the vector phi operands will be
// set at the end of vector code generation.
VPBasicBlock *Parent = getParent();
VPRegionBlock *LoopRegion = Parent->getEnclosingLoopRegion();
unsigned StartIdx = 0;
// For phis in header blocks of loop regions, use the index of the value
// coming from the preheader.
if (LoopRegion->getEntryBasicBlock() == Parent) {
for (unsigned I = 0; I < getNumOperands(); ++I) {
if (getIncomingBlock(I) ==
LoopRegion->getSinglePredecessor()->getExitingBasicBlock())
StartIdx = I;
}
}
Value *Op0 = State.get(getOperand(StartIdx), 0);
Value *Op0 = State.get(getOperand(0), 0);
Type *VecTy = Op0->getType();
Value *VecPhi = State.Builder.CreatePHI(VecTy, 2, "vec.phi");
State.set(this, VecPhi, 0);
Expand Down

0 comments on commit d9f8316

Please sign in to comment.