Skip to content

Commit b1c758c

Browse files
fiigiifiigii
committed
Address review comments
1 parent 72efb11 commit b1c758c

File tree

1 file changed

+85
-67
lines changed

1 file changed

+85
-67
lines changed

‎llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp‎

Lines changed: 85 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,11 @@ class StraightLineStrengthReduce {
495495
return S;
496496
}
497497

498+
bool candidatePredicate(Candidate *Basis, Candidate &C, Candidate::DKind K);
499+
500+
bool searchFrom(const CandidateDictTy::BBToCandsTy &BBToCands, Candidate &C,
501+
Candidate::DKind K);
502+
498503
// Get the nearest instruction before CI that represents the value of S,
499504
// return nullptr if no instruction is associated with S or S is not a
500505
// reusable expression.
@@ -629,7 +634,8 @@ Value *StraightLineStrengthReduce::getDelta(const Candidate &C,
629634
const Candidate &Basis,
630635
Candidate::DKind K) const {
631636
if (K == Candidate::IndexDelta) {
632-
APInt Idx = C.Index->getValue(), BasisIdx = Basis.Index->getValue();
637+
APInt Idx = C.Index->getValue();
638+
APInt BasisIdx = Basis.Index->getValue();
633639
unifyBitWidth(Idx, BasisIdx);
634640
APInt IndexDelta = Idx - BasisIdx;
635641
IntegerType *DeltaType =
@@ -664,92 +670,104 @@ bool StraightLineStrengthReduce::isSimilar(Candidate &C, Candidate &Basis,
664670
Basis.CandidateKind == C.CandidateKind;
665671
}
666672

667-
void StraightLineStrengthReduce::setBasisAndDeltaFor(Candidate &C) {
668-
auto SearchFrom = [this, &C](const CandidateDictTy::BBToCandsTy &BBToCands,
669-
auto IsTarget) -> bool {
670-
// Search dominating candidates by walking the immediate-dominator chain
671-
// from the candidate's defining block upward. Visiting blocks in this
672-
// order ensures we prefer the closest dominating basis.
673-
const BasicBlock *BB = C.Ins->getParent();
674-
while (BB) {
675-
auto It = BBToCands.find(BB);
676-
if (It != BBToCands.end())
677-
for (Candidate *Basis : reverse(It->second))
678-
if (IsTarget(Basis))
679-
return true;
680-
681-
const DomTreeNode *Node = DT->getNode(BB);
682-
if (!Node)
683-
break;
684-
Node = Node->getIDom();
685-
BB = Node ? Node->getBlock() : nullptr;
686-
}
673+
// Try to find a Delta that C can reuse Basis to rewrite.
674+
// Set C.Delta, C.Basis, and C.DeltaKind if found.
675+
// Return true if found a constant delta.
676+
// Return false if not found or the delta is not a constant.
677+
bool StraightLineStrengthReduce::candidatePredicate(Candidate *Basis,
678+
Candidate &C,
679+
Candidate::DKind K) {
680+
SmallVector<Instruction *> DropPoisonGeneratingInsts;
681+
// Ensure the IR of Basis->Ins is not more poisonous than its SCEV.
682+
if (!isSimilar(C, *Basis, K) ||
683+
!SE->canReuseInstruction(SE->getSCEV(Basis->Ins), Basis->Ins,
684+
DropPoisonGeneratingInsts))
687685
return false;
688-
};
689686

690-
auto For = [this, &C](Candidate::DKind K) {
691-
// return true if find a Basis with constant delta and stop searching,
692-
// return false if did not find a Basis or the delta is not a constant
693-
// and continue searching for a Basis with constant delta
694-
return [K, this, &C](Candidate *Basis) -> bool {
695-
SmallVector<Instruction *> DropPoisonGeneratingInsts;
696-
// Ensure the IR of Basis->Ins is not more poisonous than its SCEV.
697-
if (!isSimilar(C, *Basis, K) ||
698-
!SE->canReuseInstruction(SE->getSCEV(Basis->Ins), Basis->Ins,
699-
DropPoisonGeneratingInsts))
700-
return false;
687+
assert(DT->dominates(Basis->Ins, C.Ins));
688+
Value *Delta = getDelta(C, *Basis, K);
689+
if (!Delta)
690+
return false;
701691

702-
assert(DT->dominates(Basis->Ins, C.Ins));
703-
Value *Delta = getDelta(C, *Basis, K);
704-
if (K == Candidate::IndexDelta &&
705-
!C.isProfitableRewrite(Delta, Candidate::IndexDelta))
706-
return false;
692+
// IndexDelta rewrite is not always profitable, e.g.,
693+
// X = B + 8 * S
694+
// Y = B + S,
695+
// rewriting Y to X - 7 * S is probably a bad idea.
696+
// So, we need to check if the rewrite form's computation efficiency
697+
// is better than the original form.
698+
if (K == Candidate::IndexDelta &&
699+
!C.isProfitableRewrite(Delta, Candidate::IndexDelta))
700+
return false;
707701

708-
if (!Delta)
709-
return false;
702+
// If there is a Delta that we can reuse Basis to rewrite C,
703+
// clean up DropPoisonGeneratingInsts returned by successful
704+
// SE->canReuseInstruction()
705+
for (Instruction *I : DropPoisonGeneratingInsts)
706+
I->dropPoisonGeneratingAnnotations();
707+
708+
// Record delta if none has been found yet, or the new delta is
709+
// a constant that is better than the existing delta.
710+
if (!C.Delta || isa<ConstantInt>(Delta)) {
711+
C.Delta = Delta;
712+
C.Basis = Basis;
713+
C.DeltaKind = K;
714+
}
715+
return isa<ConstantInt>(C.Delta);
716+
}
710717

711-
// If there is a Delta that we can reuse Basis to rewrite C,
712-
// drop the poison-generating instructions of Basis to avoid
713-
// introducing poison.
714-
for (Instruction *I : DropPoisonGeneratingInsts)
715-
I->dropPoisonGeneratingAnnotations();
716-
717-
// Record delta if none has been found yet, or the new delta is
718-
// a constant that is better than the existing delta.
719-
if (!C.Delta || isa<ConstantInt>(Delta)) {
720-
C.Delta = Delta;
721-
C.Basis = Basis;
722-
C.DeltaKind = K;
723-
}
724-
return isa<ConstantInt>(C.Delta);
725-
};
726-
};
718+
// return true if find a Basis with constant delta and stop searching,
719+
// return false if did not find a Basis or the delta is not a constant
720+
// and continue searching for a Basis with constant delta
721+
bool StraightLineStrengthReduce::searchFrom(
722+
const CandidateDictTy::BBToCandsTy &BBToCands, Candidate &C,
723+
Candidate::DKind K) {
724+
725+
// Stride delta rewrite on Mul form is usually non-profitable, and Base
726+
// delta rewrite sometimes is profitable, so we do not support them on Mul.
727+
if (C.CandidateKind == Candidate::Mul && K != Candidate::IndexDelta)
728+
return false;
727729

730+
// Search dominating candidates by walking the immediate-dominator chain
731+
// from the candidate's defining block upward. Visiting blocks in this
732+
// order ensures we prefer the closest dominating basis.
733+
const BasicBlock *BB = C.Ins->getParent();
734+
while (BB) {
735+
auto It = BBToCands.find(BB);
736+
if (It != BBToCands.end())
737+
for (Candidate *Basis : reverse(It->second))
738+
if (candidatePredicate(Basis, C, K))
739+
return true;
740+
741+
const DomTreeNode *Node = DT->getNode(BB);
742+
if (!Node)
743+
break;
744+
Node = Node->getIDom();
745+
BB = Node ? Node->getBlock() : nullptr;
746+
}
747+
return false;
748+
}
749+
750+
void StraightLineStrengthReduce::setBasisAndDeltaFor(Candidate &C) {
728751
if (const auto *BaseDeltaCandidates =
729-
CandidateDict.getCandidatesWithDeltaKind(C, Candidate::BaseDelta)) {
730-
if ((C.CandidateKind != Candidate::Mul) &&
731-
SearchFrom(*BaseDeltaCandidates, For(Candidate::BaseDelta))) {
752+
CandidateDict.getCandidatesWithDeltaKind(C, Candidate::BaseDelta))
753+
if (searchFrom(*BaseDeltaCandidates, C, Candidate::BaseDelta)) {
732754
LLVM_DEBUG(dbgs() << "Found delta from Base: " << *C.Delta << "\n");
733755
return;
734756
}
735-
}
736757

737758
if (const auto *StrideDeltaCandidates =
738-
CandidateDict.getCandidatesWithDeltaKind(C, Candidate::StrideDelta)) {
739-
if ((C.CandidateKind != Candidate::Mul) &&
740-
SearchFrom(*StrideDeltaCandidates, For(Candidate::StrideDelta))) {
759+
CandidateDict.getCandidatesWithDeltaKind(C, Candidate::StrideDelta))
760+
if (searchFrom(*StrideDeltaCandidates, C, Candidate::StrideDelta)) {
741761
LLVM_DEBUG(dbgs() << "Found delta from Stride: " << *C.Delta << "\n");
742762
return;
743763
}
744-
}
745764

746765
if (const auto *IndexDeltaCandidates =
747-
CandidateDict.getCandidatesWithDeltaKind(C, Candidate::IndexDelta)) {
748-
if (SearchFrom(*IndexDeltaCandidates, For(Candidate::IndexDelta))) {
766+
CandidateDict.getCandidatesWithDeltaKind(C, Candidate::IndexDelta))
767+
if (searchFrom(*IndexDeltaCandidates, C, Candidate::IndexDelta)) {
749768
LLVM_DEBUG(dbgs() << "Found delta from Index: " << *C.Delta << "\n");
750769
return;
751770
}
752-
}
753771

754772
// If we did not find a constant delta, we might have found a variable delta
755773
if (C.Delta) {

0 commit comments

Comments
 (0)