[llvm] r329047 - [SCEV] Make computeExitLimit more simple and more powerful
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 20 19:20:32 PDT 2018
In case it wasn't clear, I have no reason to suspect the code in this
commit is wrong. We've looked at it and the code looks totally fine.
Much more likely that it is exposing some underlying bug somewhere much
like the one fixed here: https://reviews.llvm.org/D44818
On Fri, Apr 20, 2018 at 7:19 PM Chandler Carruth <chandlerc at gmail.com>
wrote:
> We're seeing a miscompile with this that is ... really frustrating to pin
> down.
>
> Specifically, it appears that if you build Clang itself using a Clang
> built after this revision (so a stage2 Clang binary in bootstrap parlance),
> and specifically build Clang itself with ASan enabled targeting PPC (no
> other architectures we test in this way appear to be impacted), and then
> use that asan+ppc stage2 Clang binary to run a specific OpenMP test
> (clang/test/OpenMP:for_simd_codegen.cpp), that test will fail. It will
> specifically fail the third RUN line with the output:
>
> error: 'error' diagnostics seen but not expected:
> (frontend): malformed or corrupted AST file: 'could not decompress
> embedded file contents: zlib error: Z_DATA_ERROR'
>
> It is possible we have a miscompiled zlib after this revision? Haven't yet
> pinned down whether the *write* of the PCH just above it wrote corrupt
> data, or the *read* failed to read perfectly correct data.
>
> Sadly, I don't have access to a PPC machine to easily reproduce this and
> test these things out. We just have an automated build that happens to use
> this configuration and spits out the failure. =/
>
> CC-ing various folks who may be able to help try to reproduce this
> usefully.
>
> Somewhat concerning is that this is the *only* failure we've seen testing
> past this revision. No other platform, no other test. Doesn't fail w/o
> ASan, etc etc. But it also is really reliable on our end.
>
> If anyone is trying to reproduce this, happy to chat with them on IRC and
> try to give the exact commands used at each stage of this.
>
> -Chandler
>
> PS: Huge props to +David Jones <dlj at google.com> for pinning down the
> cause of this.
>
> On Mon, Apr 2, 2018 at 11:00 PM Max Kazantsev via llvm-commits <
> llvm-commits at lists.llvm.org> 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/ScalarEvolution.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/ScalarEvolution/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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180421/c96c12b8/attachment.html>
More information about the llvm-commits
mailing list