Skip to content
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

[VPlan] Add VPIRBasicBlock, use to model pre-preheader. #93398

Merged
merged 15 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
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
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8606,7 +8606,7 @@ LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) {
// loop region contains a header and latch basic blocks.
VPlanPtr Plan = VPlan::createInitialVPlan(
createTripCountSCEV(Legal->getWidestInductionType(), PSE, OrigLoop),
*PSE.getSE());
*PSE.getSE(), OrigLoop->getLoopPreheader());
VPBasicBlock *HeaderVPBB = new VPBasicBlock("vector.body");
VPBasicBlock *LatchVPBB = new VPBasicBlock("vector.latch");
VPBlockUtils::insertBlockAfter(LatchVPBB, HeaderVPBB);
Expand Down Expand Up @@ -8854,7 +8854,7 @@ VPlanPtr LoopVectorizationPlanner::buildVPlan(VFRange &Range) {
// Create new empty VPlan
auto Plan = VPlan::createInitialVPlan(
createTripCountSCEV(Legal->getWidestInductionType(), PSE, OrigLoop),
*PSE.getSE());
*PSE.getSE(), OrigLoop->getLoopPreheader());

// Build hierarchical CFG
VPlanHCFGBuilder HCFGBuilder(OrigLoop, LI, *Plan);
Expand Down
39 changes: 27 additions & 12 deletions llvm/lib/Transforms/Vectorize/VPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,16 @@ VPBasicBlock::createEmptyBasicBlock(VPTransformState::CFGState &CFG) {
return NewBB;
}

void VPIRBasicBlock::execute(VPTransformState *State) {
assert(getHierarchicalPredecessors().empty() &&
"VPIRBasicBlock cannot have predecessors at the moment");
assert(getHierarchicalSuccessors().empty() &&
"VPIRBasicBlock cannot have successors");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"VPIRBasicBlock cannot have successors");
"VPIRBasicBlock cannot have successors at the moment");

(Pre-preheader should eventually have successors)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added, thanks!


State->Builder.SetInsertPoint(getWrappedBlock()->getTerminator());
executeRecipes(State, getWrappedBlock());
}

void VPBasicBlock::execute(VPTransformState *State) {
bool Replica = State->Instance && !State->Instance->isFirstIteration();
VPBasicBlock *PrevVPBB = State->CFG.PrevVPBB;
Expand Down Expand Up @@ -499,16 +509,7 @@ void VPBasicBlock::execute(VPTransformState *State) {
}

// 2. Fill the IR basic block with IR instructions.
LLVM_DEBUG(dbgs() << "LV: vectorizing VPBB:" << getName()
<< " in BB:" << NewBB->getName() << '\n');

State->CFG.VPBB2IRBB[this] = NewBB;
State->CFG.PrevVPBB = this;

for (VPRecipeBase &Recipe : Recipes)
Recipe.execute(*State);

LLVM_DEBUG(dbgs() << "LV: filled BB:" << *NewBB);
executeRecipes(State, NewBB);
}

void VPBasicBlock::dropAllReferences(VPValue *NewValue) {
Expand All @@ -521,6 +522,19 @@ void VPBasicBlock::dropAllReferences(VPValue *NewValue) {
}
}

void VPBasicBlock::executeRecipes(VPTransformState *State, BasicBlock *BB) {
LLVM_DEBUG(dbgs() << "LV: vectorizing VPBB:" << getName()
<< " in BB:" << BB->getName() << '\n');

State->CFG.VPBB2IRBB[this] = BB;
State->CFG.PrevVPBB = this;

for (VPRecipeBase &Recipe : Recipes)
Recipe.execute(*State);

LLVM_DEBUG(dbgs() << "LV: filled BB:" << *BB);
}

VPBasicBlock *VPBasicBlock::splitAt(iterator SplitAt) {
assert((SplitAt == end() || SplitAt->getParent() == this) &&
"can only split at a position in the same block");
Expand Down Expand Up @@ -769,8 +783,9 @@ VPlan::~VPlan() {
delete BackedgeTakenCount;
}

VPlanPtr VPlan::createInitialVPlan(const SCEV *TripCount, ScalarEvolution &SE) {
VPBasicBlock *Preheader = new VPBasicBlock("ph");
VPlanPtr VPlan::createInitialVPlan(const SCEV *TripCount, ScalarEvolution &SE,
BasicBlock *PH) {
VPIRBasicBlock *Preheader = new VPIRBasicBlock(PH);
VPBasicBlock *VecPreheader = new VPBasicBlock("vector.ph");
auto Plan = std::make_unique<VPlan>(Preheader, VecPreheader);
Plan->TripCount =
Expand Down
51 changes: 46 additions & 5 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ class VPBlockBase {
/// that are actually instantiated. Values of this enumeration are kept in the
/// SubclassID field of the VPBlockBase objects. They are used for concrete
/// type identification.
using VPBlockTy = enum { VPBasicBlockSC, VPRegionBlockSC };
using VPBlockTy = enum { VPBasicBlockSC, VPRegionBlockSC, VPIRBasicBlockSC };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
using VPBlockTy = enum { VPBasicBlockSC, VPRegionBlockSC, VPIRBasicBlockSC };
using VPBlockTy = enum { VPRegionBlockSC, VPBasicBlockSC, VPIRBasicBlockSC };

nit: order more logical?

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!


using VPBlocksTy = SmallVectorImpl<VPBlockBase *>;

Expand Down Expand Up @@ -2837,6 +2837,10 @@ class VPBasicBlock : public VPBlockBase {
/// The VPRecipes held in the order of output instructions to generate.
RecipeListTy Recipes;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, Recipes can be made protected, now that a subclass may want to access them.

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


protected:
VPBasicBlock(const unsigned char BlockSC, const Twine &Name = "")
: VPBlockBase(BlockSC, Name.str()) {}

public:
VPBasicBlock(const Twine &Name = "", VPRecipeBase *Recipe = nullptr)
: VPBlockBase(VPBasicBlockSC, Name.str()) {
Expand Down Expand Up @@ -2885,7 +2889,8 @@ class VPBasicBlock : public VPBlockBase {

/// Method to support type inquiry through isa, cast, and dyn_cast.
static inline bool classof(const VPBlockBase *V) {
return V->getVPBlockID() == VPBlockBase::VPBasicBlockSC;
return V->getVPBlockID() == VPBlockBase::VPBasicBlockSC ||
V->getVPBlockID() == VPBlockBase::VPIRBasicBlockSC;
}

void insert(VPRecipeBase *Recipe, iterator InsertPt) {
Expand Down Expand Up @@ -2948,12 +2953,48 @@ class VPBasicBlock : public VPBlockBase {
return NewBlock;
}

protected:
void executeRecipes(VPTransformState *State, BasicBlock *BB);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Documentation missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, thanks!


private:
/// Create an IR BasicBlock to hold the output instructions generated by this
/// VPBasicBlock, and return it. Update the CFGState accordingly.
BasicBlock *createEmptyBasicBlock(VPTransformState::CFGState &CFG);
};

/// 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.
class VPIRBasicBlock : public VPBasicBlock {
BasicBlock *WrappedBlock;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
BasicBlock *WrappedBlock;
BasicBlock *IRBasicBlock;

or IRBB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed, thanks


public:
VPIRBasicBlock(BasicBlock *WrappedBlock)
: VPBasicBlock(
VPIRBasicBlockSC,
(Twine("ir-bb<") + WrappedBlock->getName() + Twine(">")).str()),
WrappedBlock(WrappedBlock) {}

~VPIRBasicBlock() override {}

static inline bool classof(const VPBlockBase *V) {
return V->getVPBlockID() == VPBlockBase::VPIRBasicBlockSC;
}

/// The method which generates the output IR instructions that correspond to
/// this VPBasicBlock, thereby "executing" the VPlan.
void execute(VPTransformState *State) override;

VPIRBasicBlock *clone() override {
auto *NewBlock = new VPIRBasicBlock(WrappedBlock);
for (VPRecipeBase &R : *this)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: admittedly VPBasicBlock::clone() also iterates using *this rather than Recipes. Would be good to be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Recipes is private, can move to protected?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

NewBlock->appendRecipe(R.clone());
return NewBlock;
}

BasicBlock *getWrappedBlock() const { return WrappedBlock; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
BasicBlock *getWrappedBlock() const { return WrappedBlock; }
BasicBlock *getIRBasicBlock() const { return IRBB; }

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, thanks!

};

/// VPRegionBlock represents a collection of VPBasicBlocks and VPRegionBlocks
/// which form a Single-Entry-Single-Exiting subgraph of the output IR CFG.
/// A VPRegionBlock may indicate that its contents are to be replicated several
Expand Down Expand Up @@ -3142,12 +3183,12 @@ class VPlan {
~VPlan();

/// Create initial VPlan skeleton, having an "entry" VPBasicBlock (wrapping
/// original scalar pre-header) which contains SCEV expansions that need to
/// happen before the CFG is modified; a VPBasicBlock for the vector
/// original scalar pre-header \p PH) which contains SCEV expansions that need
/// to happen before the CFG is modified; a VPBasicBlock for the vector
/// pre-header, followed by a region for the vector loop, followed by the
/// middle VPBasicBlock.
static VPlanPtr createInitialVPlan(const SCEV *TripCount,
ScalarEvolution &PSE);
ScalarEvolution &PSE, BasicBlock *PH);

/// Prepare the plan for execution, setting up the required live-in values.
void prepareToExecute(Value *TripCount, Value *VectorTripCount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ define void @test_tc_less_than_16(ptr %A, i64 %N) {
; CHECK-NEXT: Live-in vp<[[VTC:%.+]]> = vector-trip-count
; CHECK-NEXT: vp<[[TC:%.+]]> = original trip-count
; CHECK-EMPTY:
; CHECK-NEXT: ph:
; CHECK-NEXT: ir-bb<entry>:
; CHECK-NEXT: EMIT vp<[[TC]]> = EXPAND SCEV (zext i4 (trunc i64 %N to i4) to i64)
; CHECK-NEXT: No successors
; CHECK-EMPTY:
Expand Down Expand Up @@ -45,7 +45,7 @@ define void @test_tc_less_than_16(ptr %A, i64 %N) {
; CHECK-NEXT: Live-in vp<[[VFxUF:%.+]]> = VF * UF
; CHECK-NEXT: vp<[[TC:%.+]]> = original trip-count
; CHECK-EMPTY:
; CHECK-NEXT: ph:
; CHECK-NEXT: ir-bb<entry>:
; CHECK-NEXT: EMIT vp<[[TC]]> = EXPAND SCEV (zext i4 (trunc i64 %N to i4) to i64)
; CHECK-NEXT: No successors
; CHECK-EMPTY:
Expand Down
14 changes: 8 additions & 6 deletions llvm/unittests/Transforms/Vectorize/VPlanTestBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,10 @@ class VPlanTestBase : public testing::Test {
assert(!verifyFunction(F) && "input function must be valid");
doAnalysis(F);

auto Plan = VPlan::createInitialVPlan(
SE->getBackedgeTakenCount(LI->getLoopFor(LoopHeader)), *SE);
VPlanHCFGBuilder HCFGBuilder(LI->getLoopFor(LoopHeader), LI.get(), *Plan);
Loop *L = LI->getLoopFor(LoopHeader);
auto Plan = VPlan::createInitialVPlan(SE->getBackedgeTakenCount(L), *SE,
L->getLoopPreheader());
VPlanHCFGBuilder HCFGBuilder(L, LI.get(), *Plan);
HCFGBuilder.buildHierarchicalCFG();
return Plan;
}
Expand All @@ -80,9 +81,10 @@ class VPlanTestBase : public testing::Test {
assert(!verifyFunction(F) && "input function must be valid");
doAnalysis(F);

auto Plan = VPlan::createInitialVPlan(
SE->getBackedgeTakenCount(LI->getLoopFor(LoopHeader)), *SE);
VPlanHCFGBuilder HCFGBuilder(LI->getLoopFor(LoopHeader), LI.get(), *Plan);
Loop *L = LI->getLoopFor(LoopHeader);
auto Plan = VPlan::createInitialVPlan(SE->getBackedgeTakenCount(L), *SE,
L->getLoopPreheader());
VPlanHCFGBuilder HCFGBuilder(L, LI.get(), *Plan);
HCFGBuilder.buildPlainCFG();
return Plan;
}
Expand Down
Loading