[llvm] r327615 - [SCEV][NFC] Remove TBB, FBB parameters from exit limit computations

Volkan Keles via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 15 10:53:09 PDT 2018



> On Mar 15, 2018, at 2:38 AM, Max Kazantsev via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: mkazantsev
> Date: Thu Mar 15 02:38:00 2018
> New Revision: 327615
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=327615&view=rev
> Log:
> [SCEV][NFC] Remove TBB, FBB parameters from exit limit computations
> 
> Methods `computeExitLimitFromCondCached` and `computeExitLimitFromCondImpl` take
> true and false branches as parameters and only use them for asserts and for identifying
> whether true/false branch belongs to the loop (which can be done once earlier). This fact
> complicates generalization of exit limit computation logic on guards because the guards
> don't have blocks to which they go in case of failure explicitly.
> 
> The motivation of this patch is that currently this part of SCEV knows nothing about guards
> and only works with explicit branches. As result, it fails to prove that a loop
> 
>  for (i = 0; i < 100; i++)
>    guard(i < 10);
> 
> exits after 10th iteration, while in the equivalent example
> 
>  for (i = 0; i < 100; i++)
>    if (i >= 10) break;
> 
> SCEV easily proves this fact. We are going to change it in near future, and this is why
> we need to make these methods operate on more abstract level.
> 
> This patch refactors this code to get rid of these parameters as meaningless and prepare
> ground for teaching these methods to work with guards as well as they work with explicit
> branching instructions.
> 
> Differential Revision: https://reviews.llvm.org/D44419
> 
> Modified:
>    llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
>    llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> 
> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=327615&r1=327614&r2=327615&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Thu Mar 15 02:38:00 2018
> @@ -1432,8 +1432,7 @@ private:
>                              bool AllowPredicates = false);
> 
>   /// Compute the number of times the backedge of the specified loop will
> -  /// execute if its exit condition were a conditional branch of ExitCond,
> -  /// TBB, and FBB.
> +  /// execute if its exit condition were a conditional branch of ExitCond.
>   ///
>   /// \p ControlsExit is true if ExitCond directly controls the exit
>   /// branch. In this case, we can assume that the loop exits only if the
> @@ -1443,15 +1442,14 @@ private:
>   /// If \p AllowPredicates is set, this call will try to use a minimal set of
>   /// SCEV predicates in order to return an exact answer.
>   ExitLimit computeExitLimitFromCond(const Loop *L, Value *ExitCond,
> -                                     BasicBlock *TBB, BasicBlock *FBB,
> -                                     bool ControlsExit,
> +                                     bool ExitIfTrue, bool ControlsExit,
>                                      bool AllowPredicates = false);
> 
>   // Helper functions for computeExitLimitFromCond to avoid exponential time
>   // complexity.
> 
>   class ExitLimitCache {
> -    // It may look like we need key on the whole (L, TBB, FBB, ControlsExit,
> +    // It may look like we need key on the whole (L, ExitIfTrue, ControlsExit,
>     // AllowPredicates) tuple, but recursive calls to
>     // computeExitLimitFromCondCached from computeExitLimitFromCondImpl only
>     // vary the in \c ExitCond and \c ControlsExit parameters.  We remember the
> @@ -1459,43 +1457,39 @@ private:
>     SmallDenseMap<PointerIntPair<Value *, 1>, ExitLimit> TripCountMap;
> 
>     const Loop *L;
> -    BasicBlock *TBB;
> -    BasicBlock *FBB;
> +    bool ExitIfTrue;

Hi Max,

Looks like ExitIfTrue is not used, I’m got: error: private field 'ExitIfTrue' is not used [-Werror,-Wunused-private-field]

Could you take a look?

Volkan

>     bool AllowPredicates;
> 
>   public:
> -    ExitLimitCache(const Loop *L, BasicBlock *TBB, BasicBlock *FBB,
> -                   bool AllowPredicates)
> -        : L(L), TBB(TBB), FBB(FBB), AllowPredicates(AllowPredicates) {}
> -
> -    Optional<ExitLimit> find(const Loop *L, Value *ExitCond, BasicBlock *TBB,
> -                             BasicBlock *FBB, bool ControlsExit,
> -                             bool AllowPredicates);
> -
> -    void insert(const Loop *L, Value *ExitCond, BasicBlock *TBB,
> -                BasicBlock *FBB, bool ControlsExit, bool AllowPredicates,
> -                const ExitLimit &EL);
> +    ExitLimitCache(const Loop *L, bool ExitIfTrue, bool AllowPredicates)
> +        : L(L), ExitIfTrue(ExitIfTrue), AllowPredicates(AllowPredicates) {}
> +
> +    Optional<ExitLimit> find(const Loop *L, Value *ExitCond, bool ExitIfTrue,
> +                             bool ControlsExit, bool AllowPredicates);
> +
> +    void insert(const Loop *L, Value *ExitCond, bool ExitIfTrue,
> +                bool ControlsExit, bool AllowPredicates, const ExitLimit &EL);
>   };
> 
>   using ExitLimitCacheTy = ExitLimitCache;
> 
>   ExitLimit computeExitLimitFromCondCached(ExitLimitCacheTy &Cache,
>                                            const Loop *L, Value *ExitCond,
> -                                           BasicBlock *TBB, BasicBlock *FBB,
> +                                           bool ExitIfTrue,
>                                            bool ControlsExit,
>                                            bool AllowPredicates);
>   ExitLimit computeExitLimitFromCondImpl(ExitLimitCacheTy &Cache, const Loop *L,
> -                                         Value *ExitCond, BasicBlock *TBB,
> -                                         BasicBlock *FBB, bool ControlsExit,
> +                                         Value *ExitCond, bool ExitIfTrue,
> +                                         bool ControlsExit,
>                                          bool AllowPredicates);
> 
>   /// Compute the number of times the backedge of the specified loop will
>   /// execute if its exit condition were a conditional branch of the ICmpInst
> -  /// ExitCond, TBB, and FBB. If AllowPredicates is set, this call will try
> +  /// ExitCond and ExitIfTrue. If AllowPredicates is set, this call will try
>   /// to use a minimal set of SCEV predicates in order to return an exact
>   /// answer.
>   ExitLimit computeExitLimitFromICmp(const Loop *L, ICmpInst *ExitCond,
> -                                     BasicBlock *TBB, BasicBlock *FBB,
> +                                     bool ExitIfTrue,
>                                      bool IsSubExpr,
>                                      bool AllowPredicates = false);
> 
> 
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=327615&r1=327614&r2=327615&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Thu Mar 15 02:38:00 2018
> @@ -6942,9 +6942,12 @@ ScalarEvolution::computeExitLimit(const
>   TerminatorInst *Term = ExitingBlock->getTerminator();
>   if (BranchInst *BI = dyn_cast<BranchInst>(Term)) {
>     assert(BI->isConditional() && "If unconditional, it can't be in loop!");
> +    bool ExitIfTrue = !L->contains(BI->getSuccessor(0));
> +    assert(ExitIfTrue == L->contains(BI->getSuccessor(1)) &&
> +           "It should have one successor in loop and one exit block!");
>     // Proceed to the next level to examine the exit condition expression.
>     return computeExitLimitFromCond(
> -        L, BI->getCondition(), BI->getSuccessor(0), BI->getSuccessor(1),
> +        L, BI->getCondition(), ExitIfTrue,
>         /*ControlsExit=*/IsOnlyExit, AllowPredicates);
>   }
> 
> @@ -6956,23 +6959,21 @@ ScalarEvolution::computeExitLimit(const
> }
> 
> ScalarEvolution::ExitLimit ScalarEvolution::computeExitLimitFromCond(
> -    const Loop *L, Value *ExitCond, BasicBlock *TBB, BasicBlock *FBB,
> +    const Loop *L, Value *ExitCond, bool ExitIfTrue,
>     bool ControlsExit, bool AllowPredicates) {
> -  ScalarEvolution::ExitLimitCacheTy Cache(L, TBB, FBB, AllowPredicates);
> -  return computeExitLimitFromCondCached(Cache, L, ExitCond, TBB, FBB,
> +  ScalarEvolution::ExitLimitCacheTy Cache(L, ExitIfTrue, AllowPredicates);
> +  return computeExitLimitFromCondCached(Cache, L, ExitCond, ExitIfTrue,
>                                         ControlsExit, AllowPredicates);
> }
> 
> Optional<ScalarEvolution::ExitLimit>
> ScalarEvolution::ExitLimitCache::find(const Loop *L, Value *ExitCond,
> -                                      BasicBlock *TBB, BasicBlock *FBB,
> -                                      bool ControlsExit, bool AllowPredicates) {
> +                                      bool ExitIfTrue, bool ControlsExit,
> +                                      bool AllowPredicates) {
>   (void)this->L;
> -  (void)this->TBB;
> -  (void)this->FBB;
>   (void)this->AllowPredicates;
> 
> -  assert(this->L == L && this->TBB == TBB && this->FBB == FBB &&
> +  assert(this->L == L && this->ExitIfTrue == ExitIfTrue &&
>          this->AllowPredicates == AllowPredicates &&
>          "Variance in assumed invariant key components!");
>   auto Itr = TripCountMap.find({ExitCond, ControlsExit});
> @@ -6982,11 +6983,11 @@ ScalarEvolution::ExitLimitCache::find(co
> }
> 
> void ScalarEvolution::ExitLimitCache::insert(const Loop *L, Value *ExitCond,
> -                                             BasicBlock *TBB, BasicBlock *FBB,
> +                                             bool ExitIfTrue,
>                                              bool ControlsExit,
>                                              bool AllowPredicates,
>                                              const ExitLimit &EL) {
> -  assert(this->L == L && this->TBB == TBB && this->FBB == FBB &&
> +  assert(this->L == L && this->ExitIfTrue == ExitIfTrue &&
>          this->AllowPredicates == AllowPredicates &&
>          "Variance in assumed invariant key components!");
> 
> @@ -6996,33 +6997,33 @@ void ScalarEvolution::ExitLimitCache::in
> }
> 
> ScalarEvolution::ExitLimit ScalarEvolution::computeExitLimitFromCondCached(
> -    ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, BasicBlock *TBB,
> -    BasicBlock *FBB, bool ControlsExit, bool AllowPredicates) {
> +    ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, bool ExitIfTrue,
> +    bool ControlsExit, bool AllowPredicates) {
> 
>   if (auto MaybeEL =
> -          Cache.find(L, ExitCond, TBB, FBB, ControlsExit, AllowPredicates))
> +          Cache.find(L, ExitCond, ExitIfTrue, ControlsExit, AllowPredicates))
>     return *MaybeEL;
> 
> -  ExitLimit EL = computeExitLimitFromCondImpl(Cache, L, ExitCond, TBB, FBB,
> +  ExitLimit EL = computeExitLimitFromCondImpl(Cache, L, ExitCond, ExitIfTrue,
>                                               ControlsExit, AllowPredicates);
> -  Cache.insert(L, ExitCond, TBB, FBB, ControlsExit, AllowPredicates, EL);
> +  Cache.insert(L, ExitCond, ExitIfTrue, ControlsExit, AllowPredicates, EL);
>   return EL;
> }
> 
> ScalarEvolution::ExitLimit ScalarEvolution::computeExitLimitFromCondImpl(
> -    ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, BasicBlock *TBB,
> -    BasicBlock *FBB, bool ControlsExit, bool AllowPredicates) {
> +    ExitLimitCacheTy &Cache, const Loop *L, Value *ExitCond, bool ExitIfTrue,
> +    bool ControlsExit, bool AllowPredicates) {
>   // Check if the controlling expression for this loop is an And or Or.
>   if (BinaryOperator *BO = dyn_cast<BinaryOperator>(ExitCond)) {
>     if (BO->getOpcode() == Instruction::And) {
>       // Recurse on the operands of the and.
> -      bool EitherMayExit = L->contains(TBB);
> +      bool EitherMayExit = !ExitIfTrue;
>       ExitLimit EL0 = computeExitLimitFromCondCached(
> -          Cache, L, BO->getOperand(0), TBB, FBB, ControlsExit && !EitherMayExit,
> -          AllowPredicates);
> +          Cache, L, BO->getOperand(0), ExitIfTrue,
> +          ControlsExit && !EitherMayExit, AllowPredicates);
>       ExitLimit EL1 = computeExitLimitFromCondCached(
> -          Cache, L, BO->getOperand(1), TBB, FBB, ControlsExit && !EitherMayExit,
> -          AllowPredicates);
> +          Cache, L, BO->getOperand(1), ExitIfTrue,
> +          ControlsExit && !EitherMayExit, AllowPredicates);
>       const SCEV *BECount = getCouldNotCompute();
>       const SCEV *MaxBECount = getCouldNotCompute();
>       if (EitherMayExit) {
> @@ -7044,7 +7045,6 @@ ScalarEvolution::ExitLimit ScalarEvoluti
>       } else {
>         // Both conditions must be true at the same time for the loop to exit.
>         // For now, be conservative.
> -        assert(L->contains(FBB) && "Loop block has no successor in loop!");
>         if (EL0.MaxNotTaken == EL1.MaxNotTaken)
>           MaxBECount = EL0.MaxNotTaken;
>         if (EL0.ExactNotTaken == EL1.ExactNotTaken)
> @@ -7065,13 +7065,13 @@ ScalarEvolution::ExitLimit ScalarEvoluti
>     }
>     if (BO->getOpcode() == Instruction::Or) {
>       // Recurse on the operands of the or.
> -      bool EitherMayExit = L->contains(FBB);
> +      bool EitherMayExit = ExitIfTrue;
>       ExitLimit EL0 = computeExitLimitFromCondCached(
> -          Cache, L, BO->getOperand(0), TBB, FBB, ControlsExit && !EitherMayExit,
> -          AllowPredicates);
> +          Cache, L, BO->getOperand(0), ExitIfTrue,
> +          ControlsExit && !EitherMayExit, AllowPredicates);
>       ExitLimit EL1 = computeExitLimitFromCondCached(
> -          Cache, L, BO->getOperand(1), TBB, FBB, ControlsExit && !EitherMayExit,
> -          AllowPredicates);
> +          Cache, L, BO->getOperand(1), ExitIfTrue,
> +          ControlsExit && !EitherMayExit, AllowPredicates);
>       const SCEV *BECount = getCouldNotCompute();
>       const SCEV *MaxBECount = getCouldNotCompute();
>       if (EitherMayExit) {
> @@ -7093,7 +7093,6 @@ ScalarEvolution::ExitLimit ScalarEvoluti
>       } else {
>         // Both conditions must be false at the same time for the loop to exit.
>         // For now, be conservative.
> -        assert(L->contains(TBB) && "Loop block has no successor in loop!");
>         if (EL0.MaxNotTaken == EL1.MaxNotTaken)
>           MaxBECount = EL0.MaxNotTaken;
>         if (EL0.ExactNotTaken == EL1.ExactNotTaken)
> @@ -7109,12 +7108,12 @@ ScalarEvolution::ExitLimit ScalarEvoluti
>   // Proceed to the next level to examine the icmp.
>   if (ICmpInst *ExitCondICmp = dyn_cast<ICmpInst>(ExitCond)) {
>     ExitLimit EL =
> -        computeExitLimitFromICmp(L, ExitCondICmp, TBB, FBB, ControlsExit);
> +        computeExitLimitFromICmp(L, ExitCondICmp, ExitIfTrue, ControlsExit);
>     if (EL.hasFullInfo() || !AllowPredicates)
>       return EL;
> 
>     // Try again, but use SCEV predicates this time.
> -    return computeExitLimitFromICmp(L, ExitCondICmp, TBB, FBB, ControlsExit,
> +    return computeExitLimitFromICmp(L, ExitCondICmp, ExitIfTrue, ControlsExit,
>                                     /*AllowPredicates=*/true);
>   }
> 
> @@ -7123,7 +7122,7 @@ ScalarEvolution::ExitLimit ScalarEvoluti
>   // preserve the CFG and is temporarily leaving constant conditions
>   // in place.
>   if (ConstantInt *CI = dyn_cast<ConstantInt>(ExitCond)) {
> -    if (L->contains(FBB) == !CI->getZExtValue())
> +    if (ExitIfTrue == !CI->getZExtValue())
>       // The backedge is always taken.
>       return getCouldNotCompute();
>     else
> @@ -7132,19 +7131,18 @@ ScalarEvolution::ExitLimit ScalarEvoluti
>   }
> 
>   // If it's not an integer or pointer comparison then compute it the hard way.
> -  return computeExitCountExhaustively(L, ExitCond, !L->contains(TBB));
> +  return computeExitCountExhaustively(L, ExitCond, ExitIfTrue);
> }
> 
> ScalarEvolution::ExitLimit
> ScalarEvolution::computeExitLimitFromICmp(const Loop *L,
>                                           ICmpInst *ExitCond,
> -                                          BasicBlock *TBB,
> -                                          BasicBlock *FBB,
> +                                          bool ExitIfTrue,
>                                           bool ControlsExit,
>                                           bool AllowPredicates) {
>   // If the condition was exit on true, convert the condition to exit on false
>   ICmpInst::Predicate Pred;
> -  if (!L->contains(FBB))
> +  if (!ExitIfTrue)
>     Pred = ExitCond->getPredicate();
>   else
>     Pred = ExitCond->getInversePredicate();
> @@ -7226,7 +7224,7 @@ ScalarEvolution::computeExitLimitFromICm
>   }
> 
>   auto *ExhaustiveCount =
> -      computeExitCountExhaustively(L, ExitCond, !L->contains(TBB));
> +      computeExitCountExhaustively(L, ExitCond, ExitIfTrue);
> 
>   if (!isa<SCEVCouldNotCompute>(ExhaustiveCount))
>     return ExhaustiveCount;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list