Skip to content

Commit 31c6fad

Browse files
committed
[LoopIdiomVectorize] Remove redundant DomTreeUpdates
Because of how we insert most of our vector code between the original preheader and a block splitted out from it, we actually don't need most of the DTU updates as an edge deletion update is sufficient to update the DT of the said region. This is effectively a NFC.
1 parent e276cf0 commit 31c6fad

File tree

1 file changed

+17
-35
lines changed

1 file changed

+17
-35
lines changed

llvm/lib/Transforms/Vectorize/LoopIdiomVectorize.cpp

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
//===----------------------------------------------------------------------===//
4141

4242
#include "llvm/Transforms/Vectorize/LoopIdiomVectorize.h"
43+
#include "llvm/ADT/ScopeExit.h"
4344
#include "llvm/Analysis/DomTreeUpdater.h"
4445
#include "llvm/Analysis/LoopPass.h"
4546
#include "llvm/Analysis/TargetTransformInfo.h"
@@ -391,6 +392,14 @@ Value *LoopIdiomVectorize::expandFindMismatch(
391392
BasicBlock *LoopIncBlock = BasicBlock::Create(
392393
Ctx, "mismatch_loop_inc", EndBlock->getParent(), EndBlock);
393394

395+
// This is actually one of the only two DTU updates we need. The reason being
396+
// that we're splitting `mismatch_end` out of the preheader and put
397+
// most of the stuff we create later between the preheader and
398+
// `mismatch_end`. Now when DTU removes an edge, it simply recalculates
399+
// everything in between. In this case, it will be the prehedaer and
400+
// `mismatch_end`, along with the aforementioned content. Therefore we don't
401+
// need to insert additional DTU updates for new control flow edges
402+
// added in this region.
394403
DTU.applyUpdates({{DominatorTree::Insert, Preheader, MinItCheckBlock},
395404
{DominatorTree::Delete, Preheader, EndBlock}});
396405

@@ -436,10 +445,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
436445
MDBuilder(MinItCheckBr->getContext()).createBranchWeights(99, 1));
437446
Builder.Insert(MinItCheckBr);
438447

439-
DTU.applyUpdates(
440-
{{DominatorTree::Insert, MinItCheckBlock, MemCheckBlock},
441-
{DominatorTree::Insert, MinItCheckBlock, LoopPreHeaderBlock}});
442-
443448
// For each of the arrays, check the start/end addresses are on the same
444449
// page.
445450
Builder.SetInsertPoint(MemCheckBlock);
@@ -482,10 +487,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
482487
.createBranchWeights(10, 90));
483488
Builder.Insert(CombinedPageCmpCmpBr);
484489

485-
DTU.applyUpdates(
486-
{{DominatorTree::Insert, MemCheckBlock, LoopPreHeaderBlock},
487-
{DominatorTree::Insert, MemCheckBlock, VectorLoopPreheaderBlock}});
488-
489490
// Set up the vector loop preheader, i.e. calculate initial loop predicate,
490491
// zero-extend MaxLen to 64-bits, determine the number of vector elements
491492
// processed in each iteration, etc.
@@ -512,9 +513,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
512513
BranchInst *JumpToVectorLoop = BranchInst::Create(VectorLoopStartBlock);
513514
Builder.Insert(JumpToVectorLoop);
514515

515-
DTU.applyUpdates({{DominatorTree::Insert, VectorLoopPreheaderBlock,
516-
VectorLoopStartBlock}});
517-
518516
// Set up the first vector loop block by creating the PHIs, doing the vector
519517
// loads and comparing the vectors.
520518
Builder.SetInsertPoint(VectorLoopStartBlock);
@@ -542,10 +540,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
542540
VectorLoopMismatchBlock, VectorLoopIncBlock, VectorMatchHasActiveLanes);
543541
Builder.Insert(VectorEarlyExit);
544542

545-
DTU.applyUpdates(
546-
{{DominatorTree::Insert, VectorLoopStartBlock, VectorLoopMismatchBlock},
547-
{DominatorTree::Insert, VectorLoopStartBlock, VectorLoopIncBlock}});
548-
549543
// Increment the index counter and calculate the predicate for the next
550544
// iteration of the loop. We branch back to the start of the loop if there
551545
// is at least one active lane.
@@ -565,10 +559,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
565559
BranchInst::Create(VectorLoopStartBlock, EndBlock, PredHasActiveLanes);
566560
Builder.Insert(VectorLoopBranchBack);
567561

568-
DTU.applyUpdates(
569-
{{DominatorTree::Insert, VectorLoopIncBlock, VectorLoopStartBlock},
570-
{DominatorTree::Insert, VectorLoopIncBlock, EndBlock}});
571-
572562
// If we found a mismatch then we need to calculate which lane in the vector
573563
// had a mismatch and add that on to the current loop index.
574564
Builder.SetInsertPoint(VectorLoopMismatchBlock);
@@ -592,16 +582,10 @@ Value *LoopIdiomVectorize::expandFindMismatch(
592582

593583
Builder.Insert(BranchInst::Create(EndBlock));
594584

595-
DTU.applyUpdates(
596-
{{DominatorTree::Insert, VectorLoopMismatchBlock, EndBlock}});
597-
598585
// Generate code for scalar loop.
599586
Builder.SetInsertPoint(LoopPreHeaderBlock);
600587
Builder.Insert(BranchInst::Create(LoopStartBlock));
601588

602-
DTU.applyUpdates(
603-
{{DominatorTree::Insert, LoopPreHeaderBlock, LoopStartBlock}});
604-
605589
Builder.SetInsertPoint(LoopStartBlock);
606590
PHINode *IndexPhi = Builder.CreatePHI(ResType, 2, "mismatch_index");
607591
IndexPhi->addIncoming(Start, LoopPreHeaderBlock);
@@ -623,9 +607,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
623607
BranchInst *MatchCmpBr = BranchInst::Create(LoopIncBlock, EndBlock, MatchCmp);
624608
Builder.Insert(MatchCmpBr);
625609

626-
DTU.applyUpdates({{DominatorTree::Insert, LoopStartBlock, LoopIncBlock},
627-
{DominatorTree::Insert, LoopStartBlock, EndBlock}});
628-
629610
// Have we reached the maximum permitted length for the loop?
630611
Builder.SetInsertPoint(LoopIncBlock);
631612
Value *PhiInc = Builder.CreateAdd(IndexPhi, ConstantInt::get(ResType, 1), "",
@@ -636,9 +617,6 @@ Value *LoopIdiomVectorize::expandFindMismatch(
636617
BranchInst *IVCmpBr = BranchInst::Create(EndBlock, LoopStartBlock, IVCmp);
637618
Builder.Insert(IVCmpBr);
638619

639-
DTU.applyUpdates({{DominatorTree::Insert, LoopIncBlock, EndBlock},
640-
{DominatorTree::Insert, LoopIncBlock, LoopStartBlock}});
641-
642620
// In the end block we need to insert a PHI node to deal with three cases:
643621
// 1. We didn't find a mismatch in the scalar loop, so we return MaxLen.
644622
// 2. We exitted the scalar loop early due to a mismatch and need to return
@@ -679,7 +657,12 @@ void LoopIdiomVectorize::transformByteCompare(GetElementPtrInst *GEPA,
679657
BasicBlock *Header = CurLoop->getHeader();
680658
BranchInst *PHBranch = cast<BranchInst>(Preheader->getTerminator());
681659
IRBuilder<> Builder(PHBranch);
660+
661+
// Safeguard to check if we build the correct DomTree with DTU.
662+
auto CheckDTU = llvm::make_scope_exit(
663+
[this]() { assert(DT->verify() && "Ill-formed DomTree built by DTU"); });
682664
DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
665+
683666
Builder.SetCurrentDebugLocation(PHBranch->getDebugLoc());
684667

685668
// Increment the pointer if this was done before the loads in the loop.
@@ -708,6 +691,9 @@ void LoopIdiomVectorize::transformByteCompare(GetElementPtrInst *GEPA,
708691
Builder.CreateCondBr(Builder.getTrue(), CmpBB, Header);
709692
PHBranch->eraseFromParent();
710693

694+
// Previously we take care of the DTU updates between the preheader and
695+
// `mismatch_end`. Now we need to make sure edges and blocks appended after
696+
// `mismatch_end` are also being properly accounted for.
711697
BasicBlock *MismatchEnd = cast<Instruction>(ByteCmpRes)->getParent();
712698
DTU.applyUpdates({{DominatorTree::Insert, MismatchEnd, CmpBB}});
713699

@@ -717,12 +703,8 @@ void LoopIdiomVectorize::transformByteCompare(GetElementPtrInst *GEPA,
717703
if (FoundBB != EndBB) {
718704
Value *FoundCmp = Builder.CreateICmpEQ(ByteCmpRes, MaxLen);
719705
Builder.CreateCondBr(FoundCmp, EndBB, FoundBB);
720-
DTU.applyUpdates({{DominatorTree::Insert, CmpBB, FoundBB},
721-
{DominatorTree::Insert, CmpBB, EndBB}});
722-
723706
} else {
724707
Builder.CreateBr(FoundBB);
725-
DTU.applyUpdates({{DominatorTree::Insert, CmpBB, FoundBB}});
726708
}
727709

728710
auto fixSuccessorPhis = [&](BasicBlock *SuccBB) {

0 commit comments

Comments
 (0)