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

Wed Jan 23 20:50:54 PST 2019

Hi, I was able to construct a repro. The tricky thing is that it is impossible to make such IR that it would fail on one run of loopsimplifycfg. It needs at least two passes run consecutively in the loop pass manager. I’ll update the patch and merge the fix.

As for Mikael’s repro, I’ll take a look into it now.

Thank you guys for helping with this! I appreciate it a lot.

--Max

From: Jordan Rupprecht <rupprecht at google.com>
Sent: Thursday, January 24, 2019 6:54 AM
To: Mikael Holmén <mikael.holmen at ericsson.com>
Cc: Maxim Kazantsev <max.kazantsev at azul.com>; llvm-commits at lists.llvm.org
Subject: Re: [llvm] r351520 - Re-enable terminator folding in LoopSimplifyCFG: underlying bugs fixed

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<mailto: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<mailto:rupprecht at google.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
>
> Thanks for reverting it Jordan! I’ll take a look.
>
> --Max
>
> *From:*Jordan Rupprecht <rupprecht at google.com<mailto:rupprecht at google.com>
> <mailto: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>
> <mailto: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> <mailto: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>
> <mailto: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> <mailto: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> <mailto: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<mailto: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/20190124/2dbc3bbf/attachment.html>