[llvm] r329047 - [SCEV] Make computeExitLimit more simple and more powerful
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 20 20:20:08 PDT 2018
This doesn't have anything to do with ASan FWIW, that just enables -O1.
Essentially, we're seeing this test fail on a "normal" bootstrapped Clang
with -O1 (or -O2) after this revision. But weirdly, the 3-stage PPC build
bot isn't failing. Trying to figure out why next.
On Fri, Apr 20, 2018 at 7:20 PM Chandler Carruth <chandlerc at gmail.com>
wrote:
> 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/726c27ff/attachment.html>
More information about the llvm-commits
mailing list