<div dir="ltr">Hello Max,<br><br>It looks like this commit broke one of our builders:<br><br><a href="http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/19066">http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/19066</a><br>. . . <br>[ 63%] Linking CXX executable ../../bin/llvm-pdbutil<br>In file included from /Users/buildslave/as-bldslv9/lld-x86_64-darwin13/llvm.src/lib/Analysis/ScalarEvolution.cpp:61:<br>/Users/buildslave/as-bldslv9/lld-x86_64-darwin13/llvm.src/include/llvm/Analysis/ScalarEvolution.h:1460:10: error: private field 'ExitIfTrue' is not used [-Werror,-Wunused-private-field]<br> bool ExitIfTrue;<br> ^<br><br>The builder was already red and did not sent notification on this.<br>Please have a look?<br><br>Thanks<br><br>Galina<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 15, 2018 at 2:38 AM, Max Kazantsev via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: mkazantsev<br>
Date: Thu Mar 15 02:38:00 2018<br>
New Revision: 327615<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=327615&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=327615&view=rev</a><br>
Log:<br>
[SCEV][NFC] Remove TBB, FBB parameters from exit limit computations<br>
<br>
Methods `<wbr>computeExitLimitFromCondCached<wbr>` and `computeExitLimitFromCondImpl` take<br>
true and false branches as parameters and only use them for asserts and for identifying<br>
whether true/false branch belongs to the loop (which can be done once earlier). This fact<br>
complicates generalization of exit limit computation logic on guards because the guards<br>
don't have blocks to which they go in case of failure explicitly.<br>
<br>
The motivation of this patch is that currently this part of SCEV knows nothing about guards<br>
and only works with explicit branches. As result, it fails to prove that a loop<br>
<br>
for (i = 0; i < 100; i++)<br>
guard(i < 10);<br>
<br>
exits after 10th iteration, while in the equivalent example<br>
<br>
for (i = 0; i < 100; i++)<br>
if (i >= 10) break;<br>
<br>
SCEV easily proves this fact. We are going to change it in near future, and this is why<br>
we need to make these methods operate on more abstract level.<br>
<br>
This patch refactors this code to get rid of these parameters as meaningless and prepare<br>
ground for teaching these methods to work with guards as well as they work with explicit<br>
branching instructions.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D44419" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D44419</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/<wbr>Analysis/ScalarEvolution.h<br>
llvm/trunk/lib/Analysis/<wbr>ScalarEvolution.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/<wbr>Analysis/ScalarEvolution.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=327615&r1=327614&r2=327615&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/Analysis/ScalarEvolution.<wbr>h?rev=327615&r1=327614&r2=<wbr>327615&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/<wbr>Analysis/ScalarEvolution.h (original)<br>
+++ llvm/trunk/include/llvm/<wbr>Analysis/ScalarEvolution.h Thu Mar 15 02:38:00 2018<br>
@@ -1432,8 +1432,7 @@ private:<br>
bool AllowPredicates = false);<br>
<br>
/// Compute the number of times the backedge of the specified loop will<br>
- /// execute if its exit condition were a conditional branch of ExitCond,<br>
- /// TBB, and FBB.<br>
+ /// execute if its exit condition were a conditional branch of ExitCond.<br>
///<br>
/// \p ControlsExit is true if ExitCond directly controls the exit<br>
/// branch. In this case, we can assume that the loop exits only if the<br>
@@ -1443,15 +1442,14 @@ private:<br>
/// If \p AllowPredicates is set, this call will try to use a minimal set of<br>
/// SCEV predicates in order to return an exact answer.<br>
ExitLimit computeExitLimitFromCond(const Loop *L, Value *ExitCond,<br>
- BasicBlock *TBB, BasicBlock *FBB,<br>
- bool ControlsExit,<br>
+ bool ExitIfTrue, bool ControlsExit,<br>
bool AllowPredicates = false);<br>
<br>
// Helper functions for computeExitLimitFromCond to avoid exponential time<br>
// complexity.<br>
<br>
class ExitLimitCache {<br>
- // It may look like we need key on the whole (L, TBB, FBB, ControlsExit,<br>
+ // It may look like we need key on the whole (L, ExitIfTrue, ControlsExit,<br>
// AllowPredicates) tuple, but recursive calls to<br>
// computeExitLimitFromCondCached from computeExitLimitFromCondImpl only<br>
// vary the in \c ExitCond and \c ControlsExit parameters. We remember the<br>
@@ -1459,43 +1457,39 @@ private:<br>
SmallDenseMap<PointerIntPair<<wbr>Value *, 1>, ExitLimit> TripCountMap;<br>
<br>
const Loop *L;<br>
- BasicBlock *TBB;<br>
- BasicBlock *FBB;<br>
+ bool ExitIfTrue;<br>
bool AllowPredicates;<br>
<br>
public:<br>
- ExitLimitCache(const Loop *L, BasicBlock *TBB, BasicBlock *FBB,<br>
- bool AllowPredicates)<br>
- : L(L), TBB(TBB), FBB(FBB), AllowPredicates(<wbr>AllowPredicates) {}<br>
-<br>
- Optional<ExitLimit> find(const Loop *L, Value *ExitCond, BasicBlock *TBB,<br>
- BasicBlock *FBB, bool ControlsExit,<br>
- bool AllowPredicates);<br>
-<br>
- void insert(const Loop *L, Value *ExitCond, BasicBlock *TBB,<br>
- BasicBlock *FBB, bool ControlsExit, bool AllowPredicates,<br>
- const ExitLimit &EL);<br>
+ ExitLimitCache(const Loop *L, bool ExitIfTrue, bool AllowPredicates)<br>
+ : L(L), ExitIfTrue(ExitIfTrue), AllowPredicates(<wbr>AllowPredicates) {}<br>
+<br>
+ Optional<ExitLimit> find(const Loop *L, Value *ExitCond, bool ExitIfTrue,<br>
+ bool ControlsExit, bool AllowPredicates);<br>
+<br>
+ void insert(const Loop *L, Value *ExitCond, bool ExitIfTrue,<br>
+ bool ControlsExit, bool AllowPredicates, const ExitLimit &EL);<br>
};<br>
<br>
using ExitLimitCacheTy = ExitLimitCache;<br>
<br>
ExitLimit computeExitLimitFromCondCached<wbr>(ExitLimitCacheTy &Cache,<br>
const Loop *L, Value *ExitCond,<br>
- BasicBlock *TBB, BasicBlock *FBB,<br>
+ bool ExitIfTrue,<br>
bool ControlsExit,<br>
bool AllowPredicates);<br>
ExitLimit computeExitLimitFromCondImpl(<wbr>ExitLimitCacheTy &Cache, const Loop *L,<br>
- Value *ExitCond, BasicBlock *TBB,<br>
- BasicBlock *FBB, bool ControlsExit,<br>
+ Value *ExitCond, bool ExitIfTrue,<br>
+ bool ControlsExit,<br>
bool AllowPredicates);<br>
<br>
/// Compute the number of times the backedge of the specified loop will<br>
/// execute if its exit condition were a conditional branch of the ICmpInst<br>
- /// ExitCond, TBB, and FBB. If AllowPredicates is set, this call will try<br>
+ /// ExitCond and ExitIfTrue. If AllowPredicates is set, this call will try<br>
/// to use a minimal set of SCEV predicates in order to return an exact<br>
/// answer.<br>
ExitLimit computeExitLimitFromICmp(const Loop *L, ICmpInst *ExitCond,<br>
- BasicBlock *TBB, BasicBlock *FBB,<br>
+ bool ExitIfTrue,<br>
bool IsSubExpr,<br>
bool AllowPredicates = false);<br>
<br>
<br>
Modified: llvm/trunk/lib/Analysis/<wbr>ScalarEvolution.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=327615&r1=327614&r2=327615&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Analysis/ScalarEvolution.cpp?<wbr>rev=327615&r1=327614&r2=<wbr>327615&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Analysis/<wbr>ScalarEvolution.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/<wbr>ScalarEvolution.cpp Thu Mar 15 02:38:00 2018<br>
@@ -6942,9 +6942,12 @@ ScalarEvolution::<wbr>computeExitLimit(const<br>
TerminatorInst *Term = ExitingBlock->getTerminator();<br>
if (BranchInst *BI = dyn_cast<BranchInst>(Term)) {<br>
assert(BI->isConditional() && "If unconditional, it can't be in loop!");<br>
+ bool ExitIfTrue = !L->contains(BI->getSuccessor(<wbr>0));<br>
+ assert(ExitIfTrue == L->contains(BI->getSuccessor(<wbr>1)) &&<br>
+ "It should have one successor in loop and one exit block!");<br>
// Proceed to the next level to examine the exit condition expression.<br>
return computeExitLimitFromCond(<br>
- L, BI->getCondition(), BI->getSuccessor(0), BI->getSuccessor(1),<br>
+ L, BI->getCondition(), ExitIfTrue,<br>
/*ControlsExit=*/IsOnlyExit, AllowPredicates);<br>
}<br>
<br>
@@ -6956,23 +6959,21 @@ ScalarEvolution::<wbr>computeExitLimit(const<br>
}<br>
<br>
ScalarEvolution::ExitLimit ScalarEvolution::<wbr>computeExitLimitFromCond(<br>
- const Loop *L, Value *ExitCond, BasicBlock *TBB, BasicBlock *FBB,<br>
+ const Loop *L, Value *ExitCond, bool ExitIfTrue,<br>
bool ControlsExit, bool AllowPredicates) {<br>
- ScalarEvolution::<wbr>ExitLimitCacheTy Cache(L, TBB, FBB, AllowPredicates);<br>
- return computeExitLimitFromCondCached<wbr>(Cache, L, ExitCond, TBB, FBB,<br>
+ ScalarEvolution::<wbr>ExitLimitCacheTy Cache(L, ExitIfTrue, AllowPredicates);<br>
+ return computeExitLimitFromCondCached<wbr>(Cache, L, ExitCond, ExitIfTrue,<br>
ControlsExit, AllowPredicates);<br>
}<br>
<br>
Optional<ScalarEvolution::<wbr>ExitLimit><br>
ScalarEvolution::<wbr>ExitLimitCache::find(const Loop *L, Value *ExitCond,<br>
- BasicBlock *TBB, BasicBlock *FBB,<br>
- bool ControlsExit, bool AllowPredicates) {<br>
+ bool ExitIfTrue, bool ControlsExit,<br>
+ bool AllowPredicates) {<br>
(void)this->L;<br>
- (void)this->TBB;<br>
- (void)this->FBB;<br>
(void)this->AllowPredicates;<br>
<br>
- assert(this->L == L && this->TBB == TBB && this->FBB == FBB &&<br>
+ assert(this->L == L && this->ExitIfTrue == ExitIfTrue &&<br>
this->AllowPredicates == AllowPredicates &&<br>
"Variance in assumed invariant key components!");<br>
auto Itr = TripCountMap.find({ExitCond, ControlsExit});<br>
@@ -6982,11 +6983,11 @@ ScalarEvolution::<wbr>ExitLimitCache::find(co<br>
}<br>
<br>
void ScalarEvolution::<wbr>ExitLimitCache::insert(const Loop *L, Value *ExitCond,<br>
- BasicBlock *TBB, BasicBlock *FBB,<br>
+ bool ExitIfTrue,<br>
bool ControlsExit,<br>
bool AllowPredicates,<br>
const ExitLimit &EL) {<br>
- assert(this->L == L && this->TBB == TBB && this->FBB == FBB &&<br>
+ assert(this->L == L && this->ExitIfTrue == ExitIfTrue &&<br>
this->AllowPredicates == AllowPredicates &&<br>
"Variance in assumed invariant key components!");<br>
<br>
@@ -6996,33 +6997,33 @@ void ScalarEvolution::<wbr>ExitLimitCache::in<br>
}<br>
<br>
ScalarEvolution::ExitLimit ScalarEvolution::<wbr>computeExitLimitFromCondCached<wbr>(<br>
- ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, BasicBlock *TBB,<br>
- BasicBlock *FBB, bool ControlsExit, bool AllowPredicates) {<br>
+ ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, bool ExitIfTrue,<br>
+ bool ControlsExit, bool AllowPredicates) {<br>
<br>
if (auto MaybeEL =<br>
- Cache.find(L, ExitCond, TBB, FBB, ControlsExit, AllowPredicates))<br>
+ Cache.find(L, ExitCond, ExitIfTrue, ControlsExit, AllowPredicates))<br>
return *MaybeEL;<br>
<br>
- ExitLimit EL = computeExitLimitFromCondImpl(<wbr>Cache, L, ExitCond, TBB, FBB,<br>
+ ExitLimit EL = computeExitLimitFromCondImpl(<wbr>Cache, L, ExitCond, ExitIfTrue,<br>
ControlsExit, AllowPredicates);<br>
- Cache.insert(L, ExitCond, TBB, FBB, ControlsExit, AllowPredicates, EL);<br>
+ Cache.insert(L, ExitCond, ExitIfTrue, ControlsExit, AllowPredicates, EL);<br>
return EL;<br>
}<br>
<br>
ScalarEvolution::ExitLimit ScalarEvolution::<wbr>computeExitLimitFromCondImpl(<br>
- ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, BasicBlock *TBB,<br>
- BasicBlock *FBB, bool ControlsExit, bool AllowPredicates) {<br>
+ ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, bool ExitIfTrue,<br>
+ bool ControlsExit, bool AllowPredicates) {<br>
// Check if the controlling expression for this loop is an And or Or.<br>
if (BinaryOperator *BO = dyn_cast<BinaryOperator>(<wbr>ExitCond)) {<br>
if (BO->getOpcode() == Instruction::And) {<br>
// Recurse on the operands of the and.<br>
- bool EitherMayExit = L->contains(TBB);<br>
+ bool EitherMayExit = !ExitIfTrue;<br>
ExitLimit EL0 = computeExitLimitFromCondCached<wbr>(<br>
- Cache, L, BO->getOperand(0), TBB, FBB, ControlsExit && !EitherMayExit,<br>
- AllowPredicates);<br>
+ Cache, L, BO->getOperand(0), ExitIfTrue,<br>
+ ControlsExit && !EitherMayExit, AllowPredicates);<br>
ExitLimit EL1 = computeExitLimitFromCondCached<wbr>(<br>
- Cache, L, BO->getOperand(1), TBB, FBB, ControlsExit && !EitherMayExit,<br>
- AllowPredicates);<br>
+ Cache, L, BO->getOperand(1), ExitIfTrue,<br>
+ ControlsExit && !EitherMayExit, AllowPredicates);<br>
const SCEV *BECount = getCouldNotCompute();<br>
const SCEV *MaxBECount = getCouldNotCompute();<br>
if (EitherMayExit) {<br>
@@ -7044,7 +7045,6 @@ ScalarEvolution::ExitLimit ScalarEvoluti<br>
} else {<br>
// Both conditions must be true at the same time for the loop to exit.<br>
// For now, be conservative.<br>
- assert(L->contains(FBB) && "Loop block has no successor in loop!");<br>
if (EL0.MaxNotTaken == EL1.MaxNotTaken)<br>
MaxBECount = EL0.MaxNotTaken;<br>
if (EL0.ExactNotTaken == EL1.ExactNotTaken)<br>
@@ -7065,13 +7065,13 @@ ScalarEvolution::ExitLimit ScalarEvoluti<br>
}<br>
if (BO->getOpcode() == Instruction::Or) {<br>
// Recurse on the operands of the or.<br>
- bool EitherMayExit = L->contains(FBB);<br>
+ bool EitherMayExit = ExitIfTrue;<br>
ExitLimit EL0 = computeExitLimitFromCondCached<wbr>(<br>
- Cache, L, BO->getOperand(0), TBB, FBB, ControlsExit && !EitherMayExit,<br>
- AllowPredicates);<br>
+ Cache, L, BO->getOperand(0), ExitIfTrue,<br>
+ ControlsExit && !EitherMayExit, AllowPredicates);<br>
ExitLimit EL1 = computeExitLimitFromCondCached<wbr>(<br>
- Cache, L, BO->getOperand(1), TBB, FBB, ControlsExit && !EitherMayExit,<br>
- AllowPredicates);<br>
+ Cache, L, BO->getOperand(1), ExitIfTrue,<br>
+ ControlsExit && !EitherMayExit, AllowPredicates);<br>
const SCEV *BECount = getCouldNotCompute();<br>
const SCEV *MaxBECount = getCouldNotCompute();<br>
if (EitherMayExit) {<br>
@@ -7093,7 +7093,6 @@ ScalarEvolution::ExitLimit ScalarEvoluti<br>
} else {<br>
// Both conditions must be false at the same time for the loop to exit.<br>
// For now, be conservative.<br>
- assert(L->contains(TBB) && "Loop block has no successor in loop!");<br>
if (EL0.MaxNotTaken == EL1.MaxNotTaken)<br>
MaxBECount = EL0.MaxNotTaken;<br>
if (EL0.ExactNotTaken == EL1.ExactNotTaken)<br>
@@ -7109,12 +7108,12 @@ ScalarEvolution::ExitLimit ScalarEvoluti<br>
// Proceed to the next level to examine the icmp.<br>
if (ICmpInst *ExitCondICmp = dyn_cast<ICmpInst>(ExitCond)) {<br>
ExitLimit EL =<br>
- computeExitLimitFromICmp(L, ExitCondICmp, TBB, FBB, ControlsExit);<br>
+ computeExitLimitFromICmp(L, ExitCondICmp, ExitIfTrue, ControlsExit);<br>
if (EL.hasFullInfo() || !AllowPredicates)<br>
return EL;<br>
<br>
// Try again, but use SCEV predicates this time.<br>
- return computeExitLimitFromICmp(L, ExitCondICmp, TBB, FBB, ControlsExit,<br>
+ return computeExitLimitFromICmp(L, ExitCondICmp, ExitIfTrue, ControlsExit,<br>
/*AllowPredicates=*/true);<br>
}<br>
<br>
@@ -7123,7 +7122,7 @@ ScalarEvolution::ExitLimit ScalarEvoluti<br>
// preserve the CFG and is temporarily leaving constant conditions<br>
// in place.<br>
if (ConstantInt *CI = dyn_cast<ConstantInt>(<wbr>ExitCond)) {<br>
- if (L->contains(FBB) == !CI->getZExtValue())<br>
+ if (ExitIfTrue == !CI->getZExtValue())<br>
// The backedge is always taken.<br>
return getCouldNotCompute();<br>
else<br>
@@ -7132,19 +7131,18 @@ ScalarEvolution::ExitLimit ScalarEvoluti<br>
}<br>
<br>
// If it's not an integer or pointer comparison then compute it the hard way.<br>
- return computeExitCountExhaustively(<wbr>L, ExitCond, !L->contains(TBB));<br>
+ return computeExitCountExhaustively(<wbr>L, ExitCond, ExitIfTrue);<br>
}<br>
<br>
ScalarEvolution::ExitLimit<br>
ScalarEvolution::<wbr>computeExitLimitFromICmp(const Loop *L,<br>
ICmpInst *ExitCond,<br>
- BasicBlock *TBB,<br>
- BasicBlock *FBB,<br>
+ bool ExitIfTrue,<br>
bool ControlsExit,<br>
bool AllowPredicates) {<br>
// If the condition was exit on true, convert the condition to exit on false<br>
ICmpInst::Predicate Pred;<br>
- if (!L->contains(FBB))<br>
+ if (!ExitIfTrue)<br>
Pred = ExitCond->getPredicate();<br>
else<br>
Pred = ExitCond->getInversePredicate(<wbr>);<br>
@@ -7226,7 +7224,7 @@ ScalarEvolution::<wbr>computeExitLimitFromICm<br>
}<br>
<br>
auto *ExhaustiveCount =<br>
- computeExitCountExhaustively(<wbr>L, ExitCond, !L->contains(TBB));<br>
+ computeExitCountExhaustively(<wbr>L, ExitCond, ExitIfTrue);<br>
<br>
if (!isa<SCEVCouldNotCompute>(<wbr>ExhaustiveCount))<br>
return ExhaustiveCount;<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>