[llvm] r329047 - [SCEV] Make computeExitLimit more simple and more powerful
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 23 02:22:33 PDT 2018
On 04/23/2018 11:16 AM, Maxim Kazantsev wrote:
> 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!
Sounds good!
Thanks,
Mikael
>
> 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