[llvm] r351520 - Re-enable terminator folding in LoopSimplifyCFG: underlying bugs fixed

Wed Jan 23 15:53:38 PST 2019

I tested 5 different internal targets that had all broken with this
optimization, and they now pass with this patch -- so LGTM from us, but
please take a look at Mikael's repro too.

These are all from c/c++ compilations; I'm not able to get them into an .ll
that reproduces the crash. I mean, I can run -S -emit-llvm on the .c file I
gave you, but that .ll doesn't crash when passed through opt. I'll poke a
bit more at it but I don't expect to come up with anything.

On Wed, Jan 23, 2019 at 6:29 AM Mikael Holmén <mikael.holmen at ericsson.com>
wrote:

> Hi,
>
> We have also found cases that crashes with the enabled term folding,
> that are not fixed with D57095.
>
> E.g.
>
> opt -S -o - bbi-23408.ll -loop-simplifycfg
>
> which yields
>
> opt: ../lib/Analysis/LoopInfo.cpp:460: void (anonymous
> namespace)::UnloopUpdater::updateBlockParents(): Assertion `(NL !=
> &Unloop && (!NL || NL->contains(&Unloop))) && "uninitialized successor"'
> failed.
> Stack dump:
> 0.      Program arguments: build-all/bin/opt -S -o - bbi-23408.ll
> -loop-simplifycfg
> 1.      Running pass 'Function Pass Manager' on module 'bbi-23408.ll'.
> 2.      Running pass 'Loop Pass Manager' on function '@f1'
> 3.      Running pass 'Simplify loop CFG' on basic block '%bb3'
>   #0 0x000000000229af54 PrintStackTraceSignalHandler(void*)
> (build-all/bin/opt+0x229af54)
>   #1 0x0000000002298f60 llvm::sys::RunSignalHandlers()
> (build-all/bin/opt+0x2298f60)
>   #2 0x000000000229b2b8 SignalHandler(int) (build-all/bin/opt+0x229b2b8)
>   #3 0x00007fe1fe4cb330 __restore_rt
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
>   #4 0x00007fe1fd0bac37 gsignal
>
> /build/eglibc-ripdx6/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0
>   #5 0x00007fe1fd0be028 abort
> /build/eglibc-ripdx6/eglibc-2.19/stdlib/abort.c:91:0
>   #6 0x00007fe1fd0b3bf6 __assert_fail_base
> /build/eglibc-ripdx6/eglibc-2.19/assert/assert.c:92:0
>   #7 0x00007fe1fd0b3ca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
>   #8 0x00000000016f3b4d llvm::LoopInfo::erase(llvm::Loop*)
> (build-all/bin/opt+0x16f3b4d)
>   #9 0x00000000020e0319 (anonymous
> namespace)::ConstantTerminatorFoldingImpl::run()
> (build-all/bin/opt+0x20e0319)
> #10 0x00000000020dc38e simplifyLoopCFG(llvm::Loop&,
> llvm::DominatorTree&, llvm::LoopInfo&, llvm::ScalarEvolution&,
> llvm::MemorySSAUpdater*) (build-all/bin/opt+0x20dc38e)
> #11 0x00000000020dce53 (anonymous
> namespace)::LoopSimplifyCFGLegacyPass::runOnLoop(llvm::Loop*,
> llvm::LPPassManager&) (build-all/bin/opt+0x20dce53)
> #12 0x00000000016fb08c
> llvm::LPPassManager::runOnFunction(llvm::Function&)
> (build-all/bin/opt+0x16fb08c)
> #13 0x0000000001ca18fd
> llvm::FPPassManager::runOnFunction(llvm::Function&)
> (build-all/bin/opt+0x1ca18fd)
> #14 0x0000000001ca1bb8 llvm::FPPassManager::runOnModule(llvm::Module&)
> (build-all/bin/opt+0x1ca1bb8)
> #15 0x0000000001ca201a llvm::legacy::PassManagerImpl::run(llvm::Module&)
> (build-all/bin/opt+0x1ca201a)
> #16 0x00000000007a073b main (build-all/bin/opt+0x7a073b)
> #17 0x00007fe1fd0a5f45 __libc_start_main
> /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:321:0
> #18 0x000000000078618d _start (build-all/bin/opt+0x78618d)
> Abort
>
> /Mikael
>
> On 1/23/19 1:59 PM, Maxim Kazantsev via llvm-commits wrote:
> > Hi Jordan,
> >
> > I’ve prepared a fix that helps this failure, it is
> > https://reviews.llvm.org/D57095
> >
> > It appeared to be surprisingly hard to create an IR repro of this
> > situation in my case. The bug is sensitive to the order in which the
> > pass manager processes loops. I am still struggling creating such. If
> > you have an IR repro, could you please share it?
> >
> > Also I’d like to ask you to check, if you still have any problems with
> > LoopSimplifyCFG’s term folding with this fix? I want to be sure that
> > there is no more failures on your side before I re-enable it back.
> >
> > Thanks,
> >
> > Max
> >
> > *From:*Maxim Kazantsev
> > *Sent:* Wednesday, January 23, 2019 1:00 PM
> > *To:* 'Jordan Rupprecht' <rupprecht at google.com>
> > *Cc:* llvm-commits at lists.llvm.org
> > *Subject:* RE: [llvm] r351520 - Re-enable terminator folding in
> > LoopSimplifyCFG: underlying bugs fixed
> >
> > Thanks for reverting it Jordan! I’ll take a look.
> >
> > --Max
> >
> > *From:*Jordan Rupprecht <rupprecht at google.com
> > <mailto:rupprecht at google.com>>
> > *Sent:* Wednesday, January 23, 2019 8:15 AM
> > *To:* Maxim Kazantsev <max.kazantsev at azul.com
> > <mailto:max.kazantsev at azul.com>>
> > *Cc:* llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > *Subject:* Re: [llvm] r351520 - Re-enable terminator folding in
> > LoopSimplifyCFG: underlying bugs fixed
> >
> > Here's the creduce + stack trace w/ a debug build:
> >
> > $ cat /tmp/repro.c
> >
> > int a, b, *c, d, e;
> >
> > void f() {
> >
> >    int g;
> >
> >    for (;;) {
> >
> >      for (; e; e = b) {
> >
> >        c = g;
> >
> >        for (; c; c = d)
> >
> >          if (a) break;
> >
> >        if (c) break;
> >
> >      }
> >
> >    }
> >
> > }
> >
> > $ clang -O1 -fexperimental-new-pass-manager -mllvm
> > -enable-loop-simplifycfg-term-folding -c /tmp/repro.c
> >
> > clang-9:
> > ~/src/llvm-project/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:63:
> > void llvm::DeleteDeadBlocks(SmallVectorImpl<llvm::BasicBlock *> &,
> > llvm::DomTreeUpdater *): Assertion
> >
> > `Dead.count(Pred) && "All predecessors must be dead!"' failed.
> >
> > ...
> >
> >   #9 0x00007f448da1723b
> > llvm::DeleteDeadBlocks(llvm::SmallVectorImpl<llvm::BasicBlock*>&,
> >
> llvm::DomTreeUpdater*) ~/src/llvm-project/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:62:
> >
> > 5
> >
> > #10 0x00007f448e5c67ab (anonymous
> > namespace)::ConstantTerminatorFoldingImpl::deleteDeadLoopBlocks()
> > ~/src/llvm-project/llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp:415:5
> >
> > On Tue, Jan 22, 2019 at 9:43 AM Jordan Rupprecht <rupprecht at google.com
> > <mailto:rupprecht at google.com>> wrote:
> >
> >     It looks like this is still causing crashes, so I've temporarily
> >     reverted this as https://reviews.llvm.org/rL351845. I hope to have a
> >     repro later today.
> >
> >     On Thu, Jan 17, 2019 at 9:01 PM Max Kazantsev via llvm-commits
> >     <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
> >     wrote:
> >
> >         Author: mkazantsev
> >         Date: Thu Jan 17 20:57:32 2019
> >         New Revision: 351520
> >
> >         URL: http://llvm.org/viewvc/llvm-project?rev=351520&view=rev
> >         Log:
> >         Re-enable terminator folding in LoopSimplifyCFG: underlying bugs
> >         fixed
> >
> >         Modified:
> >              llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> >
> >         Modified: llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> >         URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp?rev=351520&r1=351519&r2=351520&view=diff
> >
>  ==============================================================================
> >         --- llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> (original)
> >         +++ llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp Thu Jan
> >         17 20:57:32 2019
> >         @@ -42,7 +42,7 @@ using namespace llvm;
> >           #define DEBUG_TYPE "loop-simplifycfg"
> >
> >           static cl::opt<bool>
> >         EnableTermFolding("enable-loop-simplifycfg-term-folding",
> >         -                                       cl::init(false));
> >         +                                       cl::init(true));
> >
> >           STATISTIC(NumTerminatorsFolded,
> >                     "Number of terminators folded to unconditional
> >         branches");
> >
> >
> >         _______________________________________________
> >         llvm-commits mailing list
> >         llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> >         http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
> > _______________________________________________
> > 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/20190123/6e54b43e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190123/6e54b43e/attachment.bin>