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

[Quuxplusone]

Bugzilla Link	PR35296
Status	NEW
Importance	P enhancement
Reported by	Jonas Paulsson (paulsson@linux.vnet.ibm.com)
Reported on	2017-11-14 00:52:04 -0800
Last modified on	2017-11-15 00:33:58 -0800
Version	trunk
Hardware	PC Linux
CC	dberlin@dberlin.org, ditaliano@apple.com, ghoflehner@apple.com, kubakuderski@gmail.com, llvm-bugs@lists.llvm.org, michael.v.zolotukhin@gmail.com, paulsson@linux.vnet.ibm.com, spatel+llvm@rotateright.com, uweigand@de.ibm.com
Fixed by commit(s)
Attachments	tc_domtree.ll (6362 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 19419
reduced testcase

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'.

[Quuxplusone]

Attached tc_domtree.ll (6362 bytes, text/plain): reduced testcase

[Quuxplusone]

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
}

[Quuxplusone]

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
}

[Quuxplusone]

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 :)

[Quuxplusone]

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?

[Quuxplusone]

(In reply to Jakub Kuderski from comment #4)
> 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.