llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29k stars 11.95k forks source link

[LoopUnroll / DT] "DominatorTree is not up to date" if -unroll-runtime-multi-exit given #34644

Open JonPsson opened 6 years ago

JonPsson commented 6 years ago
Bugzilla Link 35296
Version trunk
OS Linux
Attachments reduced testcase
CC @kuhar,@JonPsson,@rotateright,@uweigand

Extended Description

bin/opt -S -mtriple=s390x-linux-gnu -mcpu=z13 tc_domtree.ll -O3 -unroll-runtime-multi-exit

Discovered on SPEC, merely by passing 'unroll-runtime-multi-exit'.

JonPsson commented 6 years ago

I cannot reproduce the bug on on any of the reduced testaces with r318246. Does llvm have to be compiled with expensive checks or some other flag?

This was observable yesterday on 318138. I hope you're not forgetting the -unroll-runtime-multi-exit flag, which is needed (see top of page for full line to opt).

Expensive checks /special build options are not needed.

kuhar commented 6 years ago

I cannot reproduce the bug on on any of the reduced testaces with r318246. Does llvm have to be compiled with expensive checks or some other flag?

llvmbot commented 6 years ago

The stopgap solution to this could be just fixing this very instance, but it makes me very nervous. I think LoopUnroll should migrate to the new incremental dominator API. This may be a little controversial because updating the dominator locally in place might end up being slightly faster than deferring the work to a generic API, but, even if it's the case (and it needs to be proven :) I'm willing to take the performance hit to avoid future bugs :)

llvmbot commented 6 years ago

Further reduced:

target triple = "s390x-ibm-linux" define void @​patatino() { bb: br label %tailrecurse.i tailrecurse.i: %indvars.iv.i = phi i64 [ %indvars.iv.next.i, %bb32.i ], [ undef, %bb ] switch i8 undef, label %bb32.i [ i8 1, label %bb19.i ] bb19.i: br label %wobble.exit bb32.i: %tmp34.i = icmp slt i64 %indvars.iv.i, 1 %indvars.iv.next.i = add nsw i64 %indvars.iv.i, -1 br i1 %tmp34.i, label %bb11.loopexit, label %tailrecurse.i wobble.exit: br label %bb11 bb11.loopexit: br label %bb11 bb11: ret void }

llvmbot commented 6 years ago

Reduced: $ ./opt red.ll -loop-unroll -unroll-runtime-multi-exit -o /dev/null DominatorTree is not up to date!

$ cat red.ll target triple = "s390x-ibm-linux" define void @​patatino() { bb: br label %tailrecurse.i tailrecurse.i: ; preds = %bb32.i, %bb %indvars.iv.i = phi i64 [ %indvars.iv.next.i, %bb32.i ], [ undef, %bb ] switch i8 undef, label %bb27.i [ i8 1, label %bb19.i ] bb19.i: ; preds = %tailrecurse.i, %tailrecurse.i br label %wobble.exit bb27.i: ; preds = %tailrecurse.i br label %bb32.i bb32.i: ; preds = %bb27.i %tmp34.i = icmp slt i64 %indvars.iv.i, 1 %indvars.iv.next.i = add nsw i64 %indvars.iv.i, -1 br i1 %tmp34.i, label %bb11.loopexit, label %tailrecurse.i wobble.exit: ; preds = %bb19.i br label %bb11 bb11.loopexit: ; preds = %bb32.i br label %bb11 bb11: ; preds = %bb11.loopexit, %wobble.exit unreachable }