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