[LoopVectorize] Enable more early exit vectorisation tests #117008
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3016,6 +3016,22 @@ void InnerLoopVectorizer::fixVectorizedLoop(VPTransformState &State) { | |||||
| PSE.getSE()->forgetLoop(OrigLoop); | ||||||
| PSE.getSE()->forgetBlockAndLoopDispositions(); | ||||||
|
|
||||||
| // When dealing with uncountable early exits we create middle.split blocks | ||||||
| // between the vector loop region and the exit block. These blocks need | ||||||
| // adding to any outer loop. | ||||||
| VPRegionBlock *VectorRegion = State.Plan->getVectorLoopRegion(); | ||||||
| Loop *OuterLoop = OrigLoop->getParentLoop(); | ||||||
| if (Legal->hasUncountableEarlyExit() && OuterLoop) { | ||||||
| VPBasicBlock *MiddleVPBB = State.Plan->getMiddleBlock(); | ||||||
| VPBlockBase *PredVPBB = MiddleVPBB->getSinglePredecessor(); | ||||||
| while (PredVPBB && PredVPBB != VectorRegion) { | ||||||
| BasicBlock *MiddleSplitBB = | ||||||
| State.CFG.VPBB2IRBB[cast<VPBasicBlock>(PredVPBB)]; | ||||||
| OuterLoop->addBasicBlockToLoop(MiddleSplitBB, *LI); | ||||||
| PredVPBB = PredVPBB->getSinglePredecessor(); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // After vectorization, the exit blocks of the original loop will have | ||||||
| // additional predecessors. Invalidate SCEVs for the exit phis in case SE | ||||||
| // looked through single-entry phis. | ||||||
|
|
@@ -3046,7 +3062,6 @@ void InnerLoopVectorizer::fixVectorizedLoop(VPTransformState &State) { | |||||
| for (Instruction *PI : PredicatedInstructions) | ||||||
| sinkScalarOperands(&*PI); | ||||||
|
|
||||||
| VPBasicBlock *HeaderVPBB = VectorRegion->getEntryBasicBlock(); | ||||||
| BasicBlock *HeaderBB = State.CFG.VPBB2IRBB[HeaderVPBB]; | ||||||
|
|
||||||
|
|
@@ -4123,7 +4138,8 @@ LoopVectorizationCostModel::computeMaxVF(ElementCount UserVF, unsigned UserIC) { | |||||
| // a bottom-test and a single exiting block. We'd have to handle the fact | ||||||
| // that not every instruction executes on the last iteration. This will | ||||||
| // require a lane mask which varies through the vector loop body. (TODO) | ||||||
| if (Legal->hasUncountableEarlyExit() || | ||||||
| TheLoop->getExitingBlock() != TheLoop->getLoopLatch()) { | ||||||
| // If there was a tail-folding hint/switch, but we can't fold the tail by | ||||||
| // masking, fallback to a vectorization with a scalar epilogue. | ||||||
| if (ScalarEpilogueStatus == CM_ScalarEpilogueNotNeededUsePredicate) { | ||||||
|
|
@@ -4753,7 +4769,9 @@ bool LoopVectorizationPlanner::isCandidateForEpilogueVectorization( | |||||
| // Epilogue vectorization code has not been auditted to ensure it handles | ||||||
| // non-latch exits properly. It may be fine, but it needs auditted and | ||||||
| // tested. | ||||||
| // TODO: Add support for loops with an early exit. | ||||||
| if (Legal->hasUncountableEarlyExit() || | ||||||
|
There was a problem hiding this comment. Is this needed? If there's an uncountable early exit, there won't be a single exiting block and the check below will be true (as there must be a latch) There was a problem hiding this comment. You're right, the latch should always be non-null. |
||||||
| OrigLoop->getExitingBlock() != OrigLoop->getLoopLatch()) | ||||||
| return false; | ||||||
|
|
||||||
| return true; | ||||||
|
|
@@ -5001,6 +5019,10 @@ LoopVectorizationCostModel::selectInterleaveCount(ElementCount VF, | |||||
| if (!Legal->isSafeForAnyVectorWidth()) | ||||||
| return 1; | ||||||
|
|
||||||
| // We don't attempt to perform interleaving for early exit loops. | ||||||
| if (Legal->hasUncountableEarlyExit()) | ||||||
| return 1; | ||||||
|
There was a problem hiding this comment. Could you add more details on why not? Is all that would be needed to deal with multiple parts in AnyOf & co? There was a problem hiding this comment. Yes, it's because we'd have to support multiple parts in AnyOf and I think we probably need to investigate the best approach and check that the generated code isn't horrible. |
||||||
|
|
||||||
| auto BestKnownTC = getSmallBestKnownTC(PSE, TheLoop); | ||||||
| const bool HasReductions = !Legal->getReductionVars().empty(); | ||||||
|
|
||||||
|
|
@@ -7813,11 +7835,14 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan( | |||||
| // 2.5 When vectorizing the epilogue, fix reduction and induction resume | ||||||
| // values from the additional bypass block. | ||||||
| if (VectorizingEpilogue) { | ||||||
| assert(!ILV.Legal->hasUncountableEarlyExit() && | ||||||
| "Epilogue vectorisation not yet supported with early exits"); | ||||||
| BasicBlock *BypassBlock = ILV.getAdditionalBypassBlock(); | ||||||
| for (VPRecipeBase &R : *ExitVPBB) { | ||||||
| fixReductionScalarResumeWhenVectorizingEpilog( | ||||||
| &R, State, State.CFG.VPBB2IRBB[ExitVPBB], BypassBlock); | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| BasicBlock *PH = OrigLoop->getLoopPreheader(); | ||||||
| for (const auto &[IVPhi, _] : Legal->getInductionVars()) { | ||||||
| auto *Inc = cast<PHINode>(IVPhi->getIncomingValueForBlock(PH)); | ||||||
|
|
@@ -10177,13 +10202,33 @@ bool LoopVectorizePass::processLoop(Loop *L) { | |||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| if (LVL.hasUncountableEarlyExit()) { | ||||||
| if (!EnableEarlyExitVectorization) { | ||||||
| reportVectorizationFailure("Auto-vectorization of loops with uncountable " | ||||||
| "early exit is not enabled", | ||||||
| "Auto-vectorization of loops with uncountable " | ||||||
| "early exit is not enabled", | ||||||
| "UncountableEarlyExitLoopsDisabled", ORE, L); | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| // Needed to prevent InnerLoopVectorizer::fixupIVUsers from crashing. | ||||||
|
There was a problem hiding this comment. Can you elaborate on this comment. What scenario causes |
||||||
| for (BasicBlock *BB : L->blocks()) { | ||||||
| for (Instruction &I : *BB) { | ||||||
| for (User *U : I.users()) { | ||||||
|
There was a problem hiding this comment. If the concern only relates to induction variables can you make use of There was a problem hiding this comment. Good suggestion. Done! There was a problem hiding this comment. I was a bit surprised to see those changes were needed given the existing checks in VPlan. I had a look to see why we were missing cases. Looks like we weren't properly handling exit phis with multiple operands properly, which should be fixable #120260 There was a problem hiding this comment. This is an issue I believe I described in a previous vectoriser call and at the time I mentioned I had a fix for this already. I just haven't had chance to land the fix because it depended upon this patch landing first. In the spirit of landing small, incremental patches I decided it was best to run all the vectoriser tests with the new flag to get the code defended, then follow on with incremental fixes and additional functionality. This patch holds up several other patches and I would prefer not to make this patch dependent on #120260. There was a problem hiding this comment. Do you have any concerns regarding #120260 or will the future patches rely on the IR based checks? It is also possible to share multiple stacked PR to give a preview of what changes are in the pipeline (one option is to have a PR just include all commits or do stacked PRs via user branches https://discourse.llvm.org/t/update-on-github-pull-requests/71540/146?u=fhahn) |
||||||
| Instruction *UI = cast<Instruction>(U); | ||||||
| if (!L->contains(UI)) { | ||||||
| reportVectorizationFailure( | ||||||
| "Auto-vectorization of loops with uncountable " | ||||||
| "early exit and live-outs is not yet supported", | ||||||
| "Auto-vectorization of loop with uncountable " | ||||||
| "early exit and live-outs is not yet supported", | ||||||
|
There was a problem hiding this comment. Personally I'd just present the facts and drop the "yet". There was a problem hiding this comment. Might be worth adding a variant that takes a single message instead of duplicating? There was a problem hiding this comment. Good suggestion! I've added one and I've started using it in this patch, but as a follow-on I'm happy to clean up any other cases in the vectoriser or legality code that take duplicated strings. |
||||||
| "UncountableEarlyExitLoopLiveOutsUnsupported", ORE, L); | ||||||
| return false; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Entrance to the VPlan-native vectorization path. Outer loops are processed | ||||||
|
|
@@ -10208,6 +10253,20 @@ bool LoopVectorizePass::processLoop(Loop *L) { | |||||
| if (UseInterleaved) | ||||||
| IAI.analyzeInterleaving(useMaskedInterleavedAccesses(*TTI)); | ||||||
|
|
||||||
| if (LVL.hasUncountableEarlyExit()) { | ||||||
| BasicBlock *LoopLatch = L->getLoopLatch(); | ||||||
| if (IAI.requiresScalarEpilogue() || | ||||||
| llvm::any_of(LVL.getCountableExitingBlocks(), | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| [LoopLatch](BasicBlock *BB) { return BB != LoopLatch; })) { | ||||||
| reportVectorizationFailure("Auto-vectorization of early exit loops " | ||||||
| "requiring a scalar epilogue is unsupported", | ||||||
| "Auto-vectorization of early exit loops " | ||||||
| "requiring a scalar epilogue is unsupported", | ||||||
| "UncountableEarlyExitUnsupported", ORE, L); | ||||||
| return false; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Check the function attributes and profiles to find out if this function | ||||||
| // should be optimized for size. | ||||||
| ScalarEpilogueLowering SEL = | ||||||
|
|
||||||
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.
Is this needed? If there's an uncountable early exit, there won't be a single exiting block and the check below will be true (as there must be a latch)
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.
You're right, the latch should always be non-null.