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

Galina Kistanova via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 15 13:35:02 PDT 2018


Hello Max,

It looks like this commit broke one of our builders:

http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/19066
. . .
[ 63%] Linking CXX executable ../../bin/llvm-pdbutil
In file included from
/Users/buildslave/as-bldslv9/lld-x86_64-darwin13/llvm.src/lib/Analysis/ScalarEvolution.cpp:61:
/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]
    bool ExitIfTrue;
         ^

The builder was already red and did not sent notification on this.
Please have a look?

Thanks

Galina

On Thu, 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;
>      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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180315/f625bd08/attachment.html>


More information about the llvm-commits mailing list