-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[VPlan] Move initial skeleton construction earlier (NFC). #150848
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
Changes from 5 commits
16952d7
f835229
9558817
3b51594
d2a7fce
7a79e1c
b28bb32
032d726
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -338,12 +338,6 @@ std::unique_ptr<VPlan> PlainCFGBuilder::buildPlainCFG() { | |||||
return std::move(Plan); | ||||||
} | ||||||
|
||||||
std::unique_ptr<VPlan> VPlanTransforms::buildPlainCFG(Loop *TheLoop, | ||||||
LoopInfo &LI) { | ||||||
PlainCFGBuilder Builder(TheLoop, &LI); | ||||||
return Builder.buildPlainCFG(); | ||||||
} | ||||||
|
||||||
/// Checks if \p HeaderVPB is a loop header block in the plain CFG; that is, it | ||||||
/// has exactly 2 predecessors (preheader and latch), where the block | ||||||
/// dominates the latch and the preheader dominates the block. If it is a | ||||||
|
@@ -459,10 +453,8 @@ static void addCanonicalIVRecipes(VPlan &Plan, VPBasicBlock *HeaderVPBB, | |||||
LatchDL); | ||||||
} | ||||||
|
||||||
void VPlanTransforms::prepareForVectorization( | ||||||
VPlan &Plan, Type *InductionTy, PredicatedScalarEvolution &PSE, | ||||||
bool RequiresScalarEpilogueCheck, bool TailFolded, Loop *TheLoop, | ||||||
DebugLoc IVDL, bool HasUncountableEarlyExit, VFRange &Range) { | ||||||
static void addInitialSkeleton(VPlan &Plan, Type *InductionTy, DebugLoc IVDL, | ||||||
PredicatedScalarEvolution &PSE, Loop *TheLoop) { | ||||||
VPDominatorTree VPDT; | ||||||
VPDT.recalculate(Plan); | ||||||
|
||||||
|
@@ -488,12 +480,55 @@ void VPlanTransforms::prepareForVectorization( | |||||
|
||||||
addCanonicalIVRecipes(Plan, HeaderVPBB, LatchVPBB, InductionTy, IVDL); | ||||||
|
||||||
[[maybe_unused]] bool HandledUncountableEarlyExit = false; | ||||||
// Create SCEV and VPValue for the trip count. | ||||||
// We use the symbolic max backedge-taken-count, which works also when | ||||||
// vectorizing loops with uncountable early exits. | ||||||
const SCEV *BackedgeTakenCountSCEV = PSE.getSymbolicMaxBackedgeTakenCount(); | ||||||
assert(!isa<SCEVCouldNotCompute>(BackedgeTakenCountSCEV) && | ||||||
"Invalid backedge-taken count"); | ||||||
ScalarEvolution &SE = *PSE.getSE(); | ||||||
const SCEV *TripCount = SE.getTripCountFromExitCount(BackedgeTakenCountSCEV, | ||||||
InductionTy, TheLoop); | ||||||
Plan.setTripCount( | ||||||
vputils::getOrCreateVPValueForSCEVExpr(Plan, TripCount, SE)); | ||||||
|
||||||
VPBasicBlock *ScalarPH = Plan.createVPBasicBlock("scalar.ph"); | ||||||
VPBlockUtils::connectBlocks(ScalarPH, Plan.getScalarHeader()); | ||||||
|
||||||
// The connection order corresponds to the operands of the conditional branch, | ||||||
// with the middle block already connected to the exit block. | ||||||
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH); | ||||||
// Also connect the entry block to the scalar preheader. | ||||||
// TODO: Also introduce a branch recipe together with the minimum trip count | ||||||
// check. | ||||||
VPBlockUtils::connectBlocks(Plan.getEntry(), ScalarPH); | ||||||
Plan.getEntry()->swapSuccessors(); | ||||||
} | ||||||
|
||||||
std::unique_ptr<VPlan> | ||||||
VPlanTransforms::buildVPlan0(Loop *TheLoop, LoopInfo &LI, Type *InductionTy, | ||||||
DebugLoc IVDL, PredicatedScalarEvolution &PSE) { | ||||||
PlainCFGBuilder Builder(TheLoop, &LI); | ||||||
std::unique_ptr<VPlan> VPlan0 = Builder.buildPlainCFG(); | ||||||
addInitialSkeleton(*VPlan0, InductionTy, IVDL, PSE, TheLoop); | ||||||
return VPlan0; | ||||||
} | ||||||
|
||||||
void VPlanTransforms::handleEarlyExitsAndAddMiddleCheck( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Splitting middle block is one part of handling certain early exits.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This currently also handled connecting the exit block/scalar ph. Once the PR lands we could default to creating the branch and then replacing it with kown-true/false depending on tail-folding/requireScalarEpilogue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better split into separate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Split up thanks. |
||||||
VPlan &Plan, bool RequiresScalarEpilogueCheck, bool TailFolded, | ||||||
bool HasUncountableEarlyExit, VFRange &Range) { | ||||||
auto *MiddleVPBB = cast<VPBasicBlock>( | ||||||
Plan.getScalarHeader()->getSinglePredecessor()->getPredecessors()[0]); | ||||||
VPBlockBase *HeaderVPB = | ||||||
Plan.getEntry()->getSuccessors()[1]->getSingleSuccessor(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These seem somewhat obscure and fragile. Perhaps better fuse into a common canonical skeleton form. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, next step would be to move this forward also. |
||||||
auto *LatchVPBB = cast<VPBasicBlock>(HeaderVPB->getPredecessors()[1]); | ||||||
|
||||||
// Disconnect all early exits from the loop leaving it with a single exit from | ||||||
// the latch. Early exits that are countable are left for a scalar epilog. The | ||||||
// condition of uncountable early exits (currently at most one is supported) | ||||||
// is fused into the latch exit, and used to branch from middle block to the | ||||||
// early exit destination. | ||||||
[[maybe_unused]] bool HandledUncountableEarlyExit = false; | ||||||
for (VPIRBasicBlock *EB : Plan.getExitBlocks()) { | ||||||
for (VPBlockBase *Pred : to_vector(EB->getPredecessors())) { | ||||||
if (Pred == MiddleVPBB) | ||||||
|
@@ -502,7 +537,8 @@ void VPlanTransforms::prepareForVectorization( | |||||
assert(!HandledUncountableEarlyExit && | ||||||
"can handle exactly one uncountable early exit"); | ||||||
handleUncountableEarlyExit(cast<VPBasicBlock>(Pred), EB, Plan, | ||||||
HeaderVPBB, LatchVPBB, Range); | ||||||
cast<VPBasicBlock>(HeaderVPB), LatchVPBB, | ||||||
Range); | ||||||
HandledUncountableEarlyExit = true; | ||||||
} else { | ||||||
for (VPRecipeBase &R : EB->phis()) | ||||||
|
@@ -516,35 +552,11 @@ void VPlanTransforms::prepareForVectorization( | |||||
assert((!HasUncountableEarlyExit || HandledUncountableEarlyExit) && | ||||||
"missed an uncountable exit that must be handled"); | ||||||
|
||||||
// Create SCEV and VPValue for the trip count. | ||||||
// We use the symbolic max backedge-taken-count, which works also when | ||||||
// vectorizing loops with uncountable early exits. | ||||||
const SCEV *BackedgeTakenCountSCEV = PSE.getSymbolicMaxBackedgeTakenCount(); | ||||||
assert(!isa<SCEVCouldNotCompute>(BackedgeTakenCountSCEV) && | ||||||
"Invalid loop count"); | ||||||
ScalarEvolution &SE = *PSE.getSE(); | ||||||
const SCEV *TripCount = SE.getTripCountFromExitCount(BackedgeTakenCountSCEV, | ||||||
InductionTy, TheLoop); | ||||||
Plan.setTripCount( | ||||||
vputils::getOrCreateVPValueForSCEVExpr(Plan, TripCount, SE)); | ||||||
|
||||||
VPBasicBlock *ScalarPH = Plan.createVPBasicBlock("scalar.ph"); | ||||||
VPBlockUtils::connectBlocks(ScalarPH, Plan.getScalarHeader()); | ||||||
|
||||||
// The connection order corresponds to the operands of the conditional branch, | ||||||
// with the middle block already connected to the exit block. | ||||||
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH); | ||||||
// Also connect the entry block to the scalar preheader. | ||||||
// TODO: Also introduce a branch recipe together with the minimum trip count | ||||||
// check. | ||||||
VPBlockUtils::connectBlocks(Plan.getEntry(), ScalarPH); | ||||||
Plan.getEntry()->swapSuccessors(); | ||||||
|
||||||
// If MiddleVPBB has a single successor then the original loop does not exit | ||||||
// via the latch and the single successor must be the scalar preheader. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Independent/follow-up: this refers to how LV handles loops whose latch does not exit, i.e., by requiring a scalar epilogue. Better handle all such cases consistently - either by wiring middle block to scalar preheader only as here, or by case 1 below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, this should be taken care of by consistently emitting a check if VectorTC == TC. This is the most general, the others are optimizations of cases where we know the check is either always false or true. |
||||||
// There's no need to add a runtime check to MiddleVPBB. | ||||||
if (MiddleVPBB->getNumSuccessors() == 1) { | ||||||
assert(MiddleVPBB->getSingleSuccessor() == ScalarPH && | ||||||
assert(MiddleVPBB->getSingleSuccessor() == Plan.getScalarPreheader() && | ||||||
"must have ScalarPH as single successor"); | ||||||
return; | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -54,21 +54,27 @@ struct VPlanTransforms { | |||||
verifyVPlanIsValid(Plan); | ||||||
} | ||||||
|
||||||
LLVM_ABI_FOR_TEST static std::unique_ptr<VPlan> buildPlainCFG(Loop *TheLoop, | ||||||
LoopInfo &LI); | ||||||
|
||||||
/// Prepare the plan for vectorization. It will introduce a dedicated | ||||||
/// VPBasicBlock for the vector pre-header as well as a VPBasicBlock as exit | ||||||
/// block of the main vector loop (middle.block). If a check is needed to | ||||||
/// Create a base VPlan0, serving as the common starting point for all later | ||||||
/// candidates. It consists of an initial plain CFG loop with loop blocks from | ||||||
/// \p TheLoop being directly translated to VPBasicBlocks with VPInstruction | ||||||
/// corresponding to the input IR. | ||||||
/// | ||||||
/// The created loop is wrapped into an initial skeleton to facilitate | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done thanks |
||||||
/// vectorization, consisting of a vector pre-header, a exit block for the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done thanks |
||||||
/// main vector loop (middle.block) and a new block as preheader of the scalar | ||||||
/// loop (scalar.ph). It also adds a canonical IV and its increment, using \p | ||||||
/// InductionTy and \p IVDL, and creates a VPValue expression for the original | ||||||
/// trip count. | ||||||
LLVM_ABI_FOR_TEST static std::unique_ptr<VPlan> | ||||||
buildVPlan0(Loop *TheLoop, LoopInfo &LI, Type *InductionTy, DebugLoc IVDL, | ||||||
PredicatedScalarEvolution &PSE); | ||||||
|
||||||
/// Update \p Plan to account for all early exits. If a check is needed to | ||||||
/// guard executing the scalar epilogue loop, it will be added to the middle | ||||||
/// block, together with VPBasicBlocks for the scalar preheader and exit | ||||||
/// blocks. \p InductionTy is the type of the canonical induction and used for | ||||||
/// related values, like the trip count expression. It also creates a VPValue | ||||||
/// expression for the original trip count. | ||||||
LLVM_ABI_FOR_TEST static void prepareForVectorization( | ||||||
VPlan &Plan, Type *InductionTy, PredicatedScalarEvolution &PSE, | ||||||
bool RequiresScalarEpilogueCheck, bool TailFolded, Loop *TheLoop, | ||||||
DebugLoc IVDL, bool HasUncountableExit, VFRange &Range); | ||||||
/// block. | ||||||
LLVM_ABI_FOR_TEST static void handleEarlyExitsAndAddMiddleCheck( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LLVM_ABI_FOR_TEST? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this has been a recent addition, and now all symbols not to be exported to the public LLVM API but needed for unit tests need to be marked as such. It looks like this was for better supporting shared libraries with symbols hidden by default: 4e2efa5 |
||||||
VPlan &Plan, bool RequiresScalarEpilogueCheck, bool TailFolded, | ||||||
bool HasUncountableExit, VFRange &Range); | ||||||
|
||||||
/// Replace loops in \p Plan's flat CFG with VPRegionBlocks, turning \p Plan's | ||||||
/// flat CFG into a hierarchical CFG. | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential follow-up: would be good to complete the common skeleton with a canonical representation of early exits and middle check, to be specialized per each VF range according to its RequiresScalarEpilogueCheck (fold tail is currently also common?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep taht sounds good!