[PATCH] Fix up thread_jump handling of jumps with clobbers (PR rtl-optimization/88904)

[PATCH] Fix up thread_jump handling of jumps with clobbers (PR rtl-optimization/88904)

[Jakub Jelinek]

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?

2019-01-21  Jakub Jelinek  <jakub@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

Re: [PATCH] Fix up thread_jump handling of jumps with clobbers (PR rtl-optimization/88904)

[Richard Biener]

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  <jakub@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)