Jakub Jelinek <ja...@redhat.com> writes:
> Hi!
>
> The following testcase used to be miscompiled by RTL jump threading
> on s390x-linux with -march=zEC12 at -O2 before r269302 which made it latent.
> The problem is we had:
> (jump_insn 26 25 87 4 (parallel [
> (set (pc)
> (if_then_else (eq (reg/v:DI 5 %r5 [orig:74 e ] [74])
> (const_int 1 [0x1]))
> (label_ref 43)
> (pc)))
> (clobber (reg:CC 33 %cc))
> ]) "rh1686696.c":16:7 1263 {*cmp_and_br_signed_di}
> (int_list:REG_BR_PROB 118111604 (nil))
> -> 43)
> as the sole non-note non-code_label insn in basic block 4, and
> (jump_insn 68 67 134 12 (parallel [
> (set (pc)
> (if_then_else (ne (reg/v:DI 5 %r5 [orig:74 e ] [74])
> (const_int 1 [0x1]))
> (label_ref:DI 23)
> (pc)))
> (set (reg/v:DI 5 %r5 [orig:74 e ] [74])
> (plus:DI (reg/v:DI 5 %r5 [orig:74 e ] [74])
> (const_int -1 [0xffffffffffffffff])))
> (clobber (scratch:DI))
> (clobber (reg:CC 33 %cc))
> ]) "rh1686696.c":13:3 1922 {doloop_di}
> (int_list:REG_BR_PROB 904381916 (nil))
> -> 23)
> as the sole non-note non-code_label insn in basic block 12, the doloop
> conditional jump branching to the bb with the above jump_insn 26.
> thread_jump sees one condition being %r5 == 1 and the other %r5 != 1,
> performs the cselib assisted checks if the b sequence is really a noop
> (but that starts with the original complete bb as a base and only does
> nonequal processing in the e->src bb) and decided to change insn 68
> to jump after the jump_insn 26 because the comparison of %r5 against 1
> has been done already.
>
> It didn't take into account that %r5 has been modified in the doloop_di
> insn though, so the conditions are comparing different values.
>
> I have thought about different approaches, below is the simplest one,
> just punt if modified_in_p. Another possibility would be to handle the
> BB_END (e->src) insn in the second rather than first loop and do the
> nonequal handling in there, but I couldn't figure out what kind of insns
> it would handle better than this more simple approach. It wouldn't handle
> hypothetical condjump instruction which compares one register and modifies
> another one, as it would add that register into nonequal and unless that
> register has been unconditionally set to something else, would keep it
> in nonequal and bail out.
Yeah. Looks like we'd have to do something overly complicated to
separate the sets for the final EXECUTE_IF_SET_IN_REG_SET, and at
that point it's equivalent to the simple way.
> Bootstrapped/regtested on {x86_64,i686}-linux (where it didn't check much
> because those targets don't have doloops) and on r269253 trunk (i.e. before
> the r269302 change) on {powerpc64le,s390x}-linux (which both have doloop
> insns). Ok for trunk?
>
> 2019-03-09 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/89634
> * cfgcleanup.c (thread_jump): Punt if registers mentioned in cond1
> are modified in BB_END (e->src) instruction.
>
> * gcc.c-torture/execute/pr89634.c: New test.
OK, thanks.
Richard
