[llvm] r329047 - [SCEV] Make computeExitLimit more simple and more powerful

Maxim Kazantsev via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 23 02:16:32 PDT 2018


Thanks Mikael!

As we've revealed recently, this patch starts triggering a bunch of underlying bugs that did not show up before because the code was accidentally correct. I am now working on tracking them down, one of fixes has been submitted as https://reviews.llvm.org/D45937 and I'm working on tracking down other places where it happens.

I'll take a look into your issue shortly!

Thanks,
Max

-----Original Message-----
From: Mikael Holmén [mailto:mikael.holmen at ericsson.com] 
Sent: Monday, April 23, 2018 2:38 PM
To: Maxim Kazantsev <max.kazantsev at azul.com>
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r329047 - [SCEV] Make computeExitLimit more simple and more powerful

Hi Max,

We've seen a crash that started happening with this commit. I wrote
PR37205 about it:
  https://bugs.llvm.org/show_bug.cgi?id=37205

Regards,
Mikael

On 04/03/2018 07:57 AM, Max Kazantsev via llvm-commits wrote:
> Author: mkazantsev
> Date: Mon Apr  2 22:57:19 2018
> New Revision: 329047
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=329047&view=rev
> Log:
> [SCEV] Make computeExitLimit more simple and more powerful
> 
> Current implementation of `computeExitLimit` has a big piece of code 
> the only purpose of which is to prove that after the execution of this 
> block the latch will be executed. What it currently checks is actually 
> a subset of situations where the exiting block dominates latch.
> 
> This patch replaces all these checks for simple particular cases with 
> domination check over loop's latch which is the only necessary 
> condition of taking the exiting block into consideration. This change 
> allows to calculate exact loop taken count for simple loops like
> 
>    for (int i = 0; i < 100; i++) {
>      if (cond) {...} else {...}
>      if (i > 50) break;
>      . . .
>    }
> 
> Differential Revision: https://reviews.llvm.org/D44677 Reviewed By: 
> efriedma
> 
> Modified:
>      llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>      llvm/trunk/test/Analysis/ScalarEvolution/exact_iter_count.ll
> 
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvol
> ution.cpp?rev=329047&r1=329046&r2=329047&view=diff
> ======================================================================
> ========
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Mon Apr  2 22:57:19 
> +++ 2018
> @@ -6884,63 +6884,12 @@ ScalarEvolution::computeBackedgeTakenCou
>   ScalarEvolution::ExitLimit
>   ScalarEvolution::computeExitLimit(const Loop *L, BasicBlock *ExitingBlock,
>                                         bool AllowPredicates) {
> -  // Okay, we've chosen an exiting block.  See what condition causes 
> us to exit
> -  // at this block and remember the exit block and whether all other 
> targets
> -  // lead to the loop header.
> -  bool MustExecuteLoopHeader = true;
> -  BasicBlock *Exit = nullptr;
> -  for (auto *SBB : successors(ExitingBlock))
> -    if (!L->contains(SBB)) {
> -      if (Exit) // Multiple exit successors.
> -        return getCouldNotCompute();
> -      Exit = SBB;
> -    } else if (SBB != L->getHeader()) {
> -      MustExecuteLoopHeader = false;
> -    }
> -
> -  // At this point, we know we have a conditional branch that 
> determines whether
> -  // the loop is exited.  However, we don't know if the branch is 
> executed each
> -  // time through the loop.  If not, then the execution count of the 
> branch will
> -  // not be equal to the trip count of the loop.
> -  //
> -  // Currently we check for this by checking to see if the Exit 
> branch goes to
> -  // the loop header.  If so, we know it will always execute the same 
> number of
> -  // times as the loop.  We also handle the case where the exit block 
> *is* the
> -  // loop header.  This is common for un-rotated loops.
> -  //
> -  // If both of those tests fail, walk up the unique predecessor 
> chain to the
> -  // header, stopping if there is an edge that doesn't exit the loop. 
> If the
> -  // header is reached, the execution count of the branch will be 
> equal to the
> -  // trip count of the loop.
> -  //
> -  //  More extensive analysis could be done to handle more cases here.
> -  //
> -  if (!MustExecuteLoopHeader && ExitingBlock != L->getHeader()) {
> -    // The simple checks failed, try climbing the unique predecessor chain
> -    // up to the header.
> -    bool Ok = false;
> -    for (BasicBlock *BB = ExitingBlock; BB; ) {
> -      BasicBlock *Pred = BB->getUniquePredecessor();
> -      if (!Pred)
> -        return getCouldNotCompute();
> -      TerminatorInst *PredTerm = Pred->getTerminator();
> -      for (const BasicBlock *PredSucc : PredTerm->successors()) {
> -        if (PredSucc == BB)
> -          continue;
> -        // If the predecessor has a successor that isn't BB and isn't
> -        // outside the loop, assume the worst.
> -        if (L->contains(PredSucc))
> -          return getCouldNotCompute();
> -      }
> -      if (Pred == L->getHeader()) {
> -        Ok = true;
> -        break;
> -      }
> -      BB = Pred;
> -    }
> -    if (!Ok)
> -      return getCouldNotCompute();
> -  }
> +  assert(L->contains(ExitingBlock) && "Exit count for non-loop 
> + block?");  // If our exiting block does not dominate the latch, then 
> + its connection with  // loop's exit limit may be far from trivial.
> +  const BasicBlock *Latch = L->getLoopLatch();  if (!Latch || 
> + !DT.dominates(ExitingBlock, Latch))
> +    return getCouldNotCompute();
>   
>     bool IsOnlyExit = (L->getExitingBlock() != nullptr);
>     TerminatorInst *Term = ExitingBlock->getTerminator(); @@ -6955,9 
> +6904,19 @@ ScalarEvolution::computeExitLimit(const
>           /*ControlsExit=*/IsOnlyExit, AllowPredicates);
>     }
>   
> -  if (SwitchInst *SI = dyn_cast<SwitchInst>(Term))
> +  if (SwitchInst *SI = dyn_cast<SwitchInst>(Term)) {
> +    // For switch, make sure that there is a single exit from the loop.
> +    BasicBlock *Exit = nullptr;
> +    for (auto *SBB : successors(ExitingBlock))
> +      if (!L->contains(SBB)) {
> +        if (Exit) // Multiple exit successors.
> +          return getCouldNotCompute();
> +        Exit = SBB;
> +      }
> +    assert(Exit && "Exiting block must have at least one exit");
>       return computeExitLimitFromSingleExitSwitch(L, SI, Exit,
>                                                   
> /*ControlsExit=*/IsOnlyExit);
> +  }
>   
>     return getCouldNotCompute();
>   }
> 
> Modified: llvm/trunk/test/Analysis/ScalarEvolution/exact_iter_count.ll
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvo
> lution/exact_iter_count.ll?rev=329047&r1=329046&r2=329047&view=diff
> ======================================================================
> ========
> --- llvm/trunk/test/Analysis/ScalarEvolution/exact_iter_count.ll 
> (original)
> +++ llvm/trunk/test/Analysis/ScalarEvolution/exact_iter_count.ll Mon 
> +++ Apr  2 22:57:19 2018
> @@ -25,3 +25,37 @@ exit:
>   side.exit:
>     ret void
>   }
> +
> +define void @test_02(i1 %c) {
> +
> +; CHECK-LABEL: Determining loop execution counts for: @test_02 ; 
> +CHECK-NEXT:  Loop %loop: <multiple exits> backedge-taken count is 50
> +
> +entry:
> +  br label %loop
> +
> +loop:
> +  %iv = phi i32 [ 0, %entry ], [ %iv.next, %backedge ]
> +  br i1 %c, label %if.true, label %if.false
> +
> +if.true:
> +  br label %merge
> +
> +if.false:
> +  br label %merge
> +
> +merge:
> +  %side.cond = icmp slt i32 %iv, 50
> +  br i1 %side.cond, label %backedge, label %side.exit
> +
> +backedge:
> +  %iv.next = add i32 %iv, 1
> +  %loop.cond = icmp slt i32 %iv, 100
> +  br i1 %loop.cond, label %loop, label %exit
> +
> +exit:
> +  ret void
> +
> +side.exit:
> +  ret void
> +}
> 
> 
> _______________________________________________
> 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