On Mon, 21 Jan 2019, Jakub Jelinek wrote:
> Hi!
>
> The following patch is miscompiled on arm thumb1, where there is
> (jump_insn 32 31 33 4 (parallel [
> (set (pc)
> (if_then_else (ne (zero_extract:SI (reg:SI 3 r3 [orig:127
> ret+8 ] [127])
> (const_int 1 [0x1])
> (const_int 0 [0]))
> (const_int 0 [0]))
> (label_ref 40)
> (pc)))
> (clobber (reg:SI 3 r3 [137]))
> ]) "pr88904.c":19:3 843 {*tbit_cbranch}
> (int_list:REG_BR_PROB 719407028 (nil))
> -> 40)
> instruction as BB_END (b). thread_jump sees the same condition in 2 of
> these jumps, originally each one is comparing a different value in r3.
> It processes with cselib's help first the bb containing the first jump and
> then goes through all the instructions in b, computing the nonequal bitmap
> which registers contain different values from the earlier bb.
> After going through the entire b, it checks that the cond2 in BB_END (b)
> doesn't use any of the registers that are different (in nonequal bitmap)
> and later on that there aren't any other differences.
> The problem is that when a register has CLOBBER, it is cleared from the
> nonequal bitmap, so, if we check cond2 after processing the above JUMP_INSN,
> we don't actually see any differences; but the condition uses the registers
> from before that JUMP_INSN, where there is a difference.
>
> The following patch fixes it by moving the cond2 verification right before
> we process the last insn in b; we still need to process it for the decision
> if there are any real differences later.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, will
> bootstrap/regtest on armv7hl-linux too, ok for trunk?
OK.
Richard.
> 2019-01-21 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/88904
> * cfgcleanup.c (thread_jump): Verify cond2 doesn't mention
> any nonequal registers before processing BB_END (b).
>
> * gcc.c-torture/execute/pr88904.c: New test.
>
> --- gcc/cfgcleanup.c.jj 2019-01-01 12:37:19.147942300 +0100
> +++ gcc/cfgcleanup.c 2019-01-21 16:45:52.576636305 +0100
> @@ -338,6 +338,13 @@ thread_jump (edge e, basic_block b)
> insn != NEXT_INSN (BB_END (b)) && !failed;
> insn = NEXT_INSN (insn))
> {
> + /* cond2 must not mention any register that is not equal to the
> + former block. Check this before processing that instruction,
> + as BB_END (b) could contain also clobbers. */
> + if (insn == BB_END (b)
> + && mentions_nonequal_regs (cond2, nonequal))
> + goto failed_exit;
> +
> if (INSN_P (insn))
> {
> rtx pat = PATTERN (insn);
> @@ -362,11 +369,6 @@ thread_jump (edge e, basic_block b)
> goto failed_exit;
> }
>
> - /* cond2 must not mention any register that is not equal to the
> - former block. */
> - if (mentions_nonequal_regs (cond2, nonequal))
> - goto failed_exit;
> -
> EXECUTE_IF_SET_IN_REG_SET (nonequal, 0, i, rsi)
> goto failed_exit;
>
> --- gcc/testsuite/gcc.c-torture/execute/pr88904.c.jj 2019-01-21
> 16:47:16.194265770 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr88904.c 2019-01-21
> 16:46:59.278543027 +0100
> @@ -0,0 +1,38 @@
> +/* PR rtl-optimization/88904 */
> +
> +volatile int v;
> +
> +__attribute__((noipa)) void
> +bar (const char *x, const char *y, int z)
> +{
> + if (!v)
> + __builtin_abort ();
> + asm volatile ("" : "+g" (x));
> + asm volatile ("" : "+g" (y));
> + asm volatile ("" : "+g" (z));
> +}
> +
> +#define my_assert(e) ((e) ? (void) 0 : bar (#e, __FILE__, __LINE__))
> +
> +typedef struct {
> + unsigned M1;
> + unsigned M2 : 1;
> + int : 0;
> + unsigned M3 : 1;
> +} S;
> +
> +S
> +foo ()
> +{
> + S result = {0, 0, 1};
> + return result;
> +}
> +
> +int
> +main ()
> +{
> + S ret = foo ();
> + my_assert (ret.M2 == 0);
> + my_assert (ret.M3 == 1);
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
