Skip to content

Commit 0c23dc2

Browse files
committed
Reapply [SCEV] Replace IsAvailableOnEntry with block disposition
This exposed an issue in SCEVExpander/LCSSA, which has been fixed in D150681. ----- As far as I understand, the IsAvailableOnEntry() function basically implements the same functionality as the properlyDominates() block disposition. The primary difference (apart from a weaker implementation) seems to be in this comment at the top: // Checks if the SCEV S is available at BB. S is considered available at BB // if S can be materialized at BB without introducing a fault. However, I don't really understand why there would be such a requirement. It's my understanding that SCEV explicitly does not care about trapping udiv instructions itself, and it's the job of SCEVExpander's isSafeToExpand() to make sure these don't get expanded if they may trap. Differential Revision: https://reviews.llvm.org/D149344
1 parent a2f7352 commit 0c23dc2

File tree

2 files changed

+3
-89
lines changed

2 files changed

+3
-89
lines changed

llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 2 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -5909,90 +5909,6 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
59095909
return nullptr;
59105910
}
59115911

5912-
// Checks if the SCEV S is available at BB. S is considered available at BB
5913-
// if S can be materialized at BB without introducing a fault.
5914-
static bool IsAvailableOnEntry(const Loop *L, DominatorTree &DT, const SCEV *S,
5915-
BasicBlock *BB) {
5916-
struct CheckAvailable {
5917-
bool TraversalDone = false;
5918-
bool Available = true;
5919-
5920-
const Loop *L = nullptr; // The loop BB is in (can be nullptr)
5921-
BasicBlock *BB = nullptr;
5922-
DominatorTree &DT;
5923-
5924-
CheckAvailable(const Loop *L, BasicBlock *BB, DominatorTree &DT)
5925-
: L(L), BB(BB), DT(DT) {}
5926-
5927-
bool setUnavailable() {
5928-
TraversalDone = true;
5929-
Available = false;
5930-
return false;
5931-
}
5932-
5933-
bool follow(const SCEV *S) {
5934-
switch (S->getSCEVType()) {
5935-
case scConstant:
5936-
case scVScale:
5937-
case scPtrToInt:
5938-
case scTruncate:
5939-
case scZeroExtend:
5940-
case scSignExtend:
5941-
case scAddExpr:
5942-
case scMulExpr:
5943-
case scUMaxExpr:
5944-
case scSMaxExpr:
5945-
case scUMinExpr:
5946-
case scSMinExpr:
5947-
case scSequentialUMinExpr:
5948-
// These expressions are available if their operand(s) is/are.
5949-
return true;
5950-
5951-
case scAddRecExpr: {
5952-
// We allow add recurrences that are on the loop BB is in, or some
5953-
// outer loop. This guarantees availability because the value of the
5954-
// add recurrence at BB is simply the "current" value of the induction
5955-
// variable. We can relax this in the future; for instance an add
5956-
// recurrence on a sibling dominating loop is also available at BB.
5957-
const auto *ARLoop = cast<SCEVAddRecExpr>(S)->getLoop();
5958-
if (L && (ARLoop == L || ARLoop->contains(L)))
5959-
return true;
5960-
5961-
return setUnavailable();
5962-
}
5963-
5964-
case scUnknown: {
5965-
// For SCEVUnknown, we check for simple dominance.
5966-
const auto *SU = cast<SCEVUnknown>(S);
5967-
Value *V = SU->getValue();
5968-
5969-
if (isa<Argument>(V))
5970-
return false;
5971-
5972-
if (isa<Instruction>(V) && DT.dominates(cast<Instruction>(V), BB))
5973-
return false;
5974-
5975-
return setUnavailable();
5976-
}
5977-
5978-
case scUDivExpr:
5979-
case scCouldNotCompute:
5980-
// We do not try to smart about these at all.
5981-
return setUnavailable();
5982-
}
5983-
llvm_unreachable("Unknown SCEV kind!");
5984-
}
5985-
5986-
bool isDone() { return TraversalDone; }
5987-
};
5988-
5989-
CheckAvailable CA(L, BB, DT);
5990-
SCEVTraversal<CheckAvailable> ST(CA);
5991-
5992-
ST.visitAll(S);
5993-
return CA.Available;
5994-
}
5995-
59965912
// Try to match a control flow sequence that branches out at BI and merges back
59975913
// at Merge into a "C ? LHS : RHS" select pattern. Return true on a successful
59985914
// match.
@@ -6030,8 +5946,6 @@ const SCEV *ScalarEvolution::createNodeFromSelectLikePHI(PHINode *PN) {
60305946
auto IsReachable =
60315947
[&](BasicBlock *BB) { return DT.isReachableFromEntry(BB); };
60325948
if (PN->getNumIncomingValues() == 2 && all_of(PN->blocks(), IsReachable)) {
6033-
const Loop *L = LI.getLoopFor(PN->getParent());
6034-
60355949
// Try to match
60365950
//
60375951
// br %cond, label %left, label %right
@@ -6052,8 +5966,8 @@ const SCEV *ScalarEvolution::createNodeFromSelectLikePHI(PHINode *PN) {
60525966

60535967
if (BI && BI->isConditional() &&
60545968
BrPHIToSelect(DT, BI, PN, Cond, LHS, RHS) &&
6055-
IsAvailableOnEntry(L, DT, getSCEV(LHS), PN->getParent()) &&
6056-
IsAvailableOnEntry(L, DT, getSCEV(RHS), PN->getParent()))
5969+
properlyDominates(getSCEV(LHS), PN->getParent()) &&
5970+
properlyDominates(getSCEV(RHS), PN->getParent()))
60575971
return createNodeForSelectOrPHI(PN, Cond, LHS, RHS);
60585972
}
60595973

llvm/test/Analysis/ScalarEvolution/logical-operations.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ define ptr @tautological_select_like_phi(i32 %tc) {
427427
; CHECK-NEXT: %iv = phi i32 [ 0, %entry ], [ %iv.next, %latch ]
428428
; CHECK-NEXT: --> {0,+,1}<nuw><nsw><%loop> U: [0,101) S: [0,101) Exits: 100 LoopDispositions: { %loop: Computable }
429429
; CHECK-NEXT: %r = phi ptr [ @constant, %truebb ], [ @another_constant, %falsebb ]
430-
; CHECK-NEXT: --> %r U: [0,-3) S: [-9223372036854775808,9223372036854775805) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
430+
; CHECK-NEXT: --> @constant U: [0,-3) S: [-9223372036854775808,9223372036854775805) Exits: @constant LoopDispositions: { %loop: Invariant }
431431
; CHECK-NEXT: %iv.next = add i32 %iv, 1
432432
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,102) S: [1,102) Exits: 101 LoopDispositions: { %loop: Computable }
433433
; CHECK-NEXT: Determining loop execution counts for: @tautological_select_like_phi

0 commit comments

Comments
 (0)