[llvm] r302876 - [IfConversion] Keep the CFG updated incrementally in IfConvertTriangle

[Tobias Grosser via llvm-commits]

On Mon, May 29, 2017, at 07:12 AM, Mikael Holmén wrote:
> Hi Tobias,
> 
> On 05/25/2017 07:02 AM, Tobias Grosser wrote:
> > Hi Michael,
> > 
> > I suspect this commit triggered a regression in the AOSP buildbot [1]
> > which we just saw now (as it was hidden by other errors):
> > 
> > You can see the error in one of its recent build logs:
> > 
> > http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/132/steps/build-aosp/logs/stdio
> > 
> > where we see the following backtrace:
> > 
> > clang++:
> > /var/lib/buildbot/slaves/hexagon-build-03/aosp/llvm.src/lib/CodeGen/MachineBasicBlock.cpp:569:
> > MachineBasicBlock::succ_iterator
> > llvm::MachineBasicBlock::removeSuccessor(succ_iterator, bool): Assertion
> > `I != Successors.end() && "Not a current successor!"' failed.
> > #0 0x00000000014cf494 PrintStackTraceSignalHandler(void*)
> > (llvm.inst/bin/clang+++0x14cf494)
> > #1 0x00000000014cf7a6 SignalHandler(int)
> > (llvm.inst/bin/clang+++0x14cf7a6)
> > #2 0x00007f4c5c043d10 __restore_rt
> > (/lib/x86_64-linux-gnu/libpthread.so.0+0x10d10)
> > #3 0x00007f4c5ac21267 gsignal
> > /build/buildd/glibc-2.21/signal/../sysdeps/unix/sysv/linux/raise.c:55:0
> > #4 0x00007f4c5ac22eca abort /build/buildd/glibc-2.21/stdlib/abort.c:91:0
> > #5 0x00007f4c5ac1a03d __assert_fail_base
> > /build/buildd/glibc-2.21/assert/assert.c:92:0
> > #6 0x00007f4c5ac1a0f2 (/lib/x86_64-linux-gnu/libc.so.6+0x2e0f2)
> > #7 0x0000000000d7829c
> > llvm::MachineBasicBlock::removeSuccessor(__gnu_cxx::__normal_iterator<llvm::MachineBasicBlock**,
> > std::vector<llvm::MachineBasicBlock*,
> > std::allocator<llvm::MachineBasicBlock*> > >, bool)
> > (llvm.inst/bin/clang+++0xd7829c)
> > #8 0x0000000000d3f317 (anonymous
> > namespace)::IfConverter::runOnMachineFunction(llvm::MachineFunction&)
> > (llvm.inst/bin/clang+++0xd3f317)
> > #9 0x0000000000d9f009
> > llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
> > (llvm.inst/bin/clang+++0xd9f009)
> > #10 0x000000000108b004
> > llvm::FPPassManager::runOnFunction(llvm::Function&)
> > (llvm.inst/bin/clang+++0x108b004)
> > #11 0x000000000108b253 llvm::FPPassManager::runOnModule(llvm::Module&)
> > (llvm.inst/bin/clang+++0x108b253)
> > #12 0x000000000108b745 llvm::legacy::PassManagerImpl::run(llvm::Module&)
> > (llvm.inst/bin/clang+++0x108b745)
> > #13 0x000000000165d3cf
> > clang::EmitBackendOutput(clang::DiagnosticsEngine&,
> > clang::HeaderSearchOptions const&, clang::CodeGenOptions const&,
> > clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout
> > const&, llvm::Module*, clang::BackendAction,
> > std::unique_ptr<llvm::raw_pwrite_stream,
> > std::default_delete<llvm::raw_pwrite_stream> >)
> > (llvm.inst/bin/clang+++0x165d3cf)
> > #14 0x0000000001e2b870
> > clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&)
> > (llvm.inst/bin/clang+++0x1e2b870)
> > #15 0x00000000020be055 clang::ParseAST(clang::Sema&, bool, bool)
> > (llvm.inst/bin/clang+++0x20be055)
> > #16 0x0000000001a1e3e8 clang::FrontendAction::Execute()
> > (llvm.inst/bin/clang+++0x1a1e3e8)
> > #17 0x00000000019e2511
> > clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
> > (llvm.inst/bin/clang+++0x19e2511)
> > #18 0x0000000001aa9045
> > clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
> > (llvm.inst/bin/clang+++0x1aa9045)
> > #19 0x00000000007e4503 cc1_main(llvm::ArrayRef<char const*>, char
> > const*, void*) (llvm.inst/bin/clang+++0x7e4503)
> > #20 0x00000000007e2af2 main (llvm.inst/bin/clang+++0x7e2af2)
> > #21 0x00007f4c5ac0ca40 __libc_start_main
> > /build/buildd/glibc-2.21/csu/libc-start.c:323:0
> > #22 0x00000000007df7e9 _start (llvm.inst/bin/clang+++0x7df7e9)
> > 
> > Eli might be able to get you a reduced test case within one of the
> > following days, but maybe this already rings a bell?
> > 
> 
> (Sorry for the late reply, we had thursday/friday off here in Sweden.)

Hope you had a relaxed weekend!
 
> Yes it rings a bell.
> 
> I'm in the process of rewriting the other parts of IfConversion as well 
> to not need to rely on RemoveExtraEdges/analyzeBranch, and during that 
> work I realized my original fix to IfConvertTriangle had a bug that 
> could lead to the above.
> 
> I intend to fix my introduced bug as well in my rewrite commit, but the 
> whole thing is taking a lot more time than I first anticipated, so I'm 
> totally fine if you want to revert r302876 in the meantime.

Thanks for the info: I reverted it for now in r304128. Looking forward
to see the if-conversion being updated! Thanks for your work there!
 
> If you can manage to get an ll-reproducer I'd be happy so I can see that 
> it's not something else than the problem I'm aware of.

I do not have access to the buildbot, but Eli might be able to help out.

Best,
Tobias