postreload: Fix up postreload combine [PR93402]
diff mbox series

Message ID 20200123182202.GC10088@tucnak
State New
Headers show
Series
  • postreload: Fix up postreload combine [PR93402]
Related show

Commit Message

Jakub Jelinek Jan. 23, 2020, 6:22 p.m. UTC
Hi!

The following testcase is miscompiled, because the postreload pass changes:
-(insn 14 13 23 2 (parallel [
-            (set (reg:DI 1 dx [94])
-                (plus:DI (reg:DI 1 dx [95])
-                    (reg:DI 5 di [92])))
-            (clobber (reg:CC 17 flags))
-        ]) "pr93402.c":8:30 186 {*adddi_1}
-     (expr_list:REG_EQUAL (plus:DI (reg:DI 5 di [92])
-            (const_int 111111111111 [0x19debd01c7]))
-        (nil)))
-(insn 23 14 25 2 (set (reg:SI 0 ax)
+(insn 23 13 25 2 (set (reg:SI 0 ax)
         (const_int 0 [0])) "pr93402.c":10:1 67 {*movsi_internal}
      (nil))
 (insn 25 23 26 2 (use (reg:SI 0 ax)) "pr93402.c":10:1 -1
      (nil))
-(insn 26 25 35 2 (use (reg:DI 1 dx)) "pr93402.c":10:1 -1
+(insn 26 25 35 2 (use (plus:DI (reg:DI 1 dx [95])
+            (reg:DI 5 di [92]))) "pr93402.c":10:1 -1
      (nil))
A USE insn is not a normal insn and verify_changes called from
apply_change_group is happy about any changes into it.
The following patch avoids this optimization if we were to change
the USE operand (this routine only changes a reg into (plus reg reg2)).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
later for release branches?

2020-01-23  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/93402
	* postreload.c (reload_combine_recognize_pattern): Don't try to adjust
	USE insns.

	* gcc.c-torture/execute/pr93402.c: New test.


	Jakub

Comments

Richard Biener Jan. 23, 2020, 7:05 p.m. UTC | #1
On January 23, 2020 7:22:02 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The following testcase is miscompiled, because the postreload pass
>changes:
>-(insn 14 13 23 2 (parallel [
>-            (set (reg:DI 1 dx [94])
>-                (plus:DI (reg:DI 1 dx [95])
>-                    (reg:DI 5 di [92])))
>-            (clobber (reg:CC 17 flags))
>-        ]) "pr93402.c":8:30 186 {*adddi_1}
>-     (expr_list:REG_EQUAL (plus:DI (reg:DI 5 di [92])
>-            (const_int 111111111111 [0x19debd01c7]))
>-        (nil)))
>-(insn 23 14 25 2 (set (reg:SI 0 ax)
>+(insn 23 13 25 2 (set (reg:SI 0 ax)
>         (const_int 0 [0])) "pr93402.c":10:1 67 {*movsi_internal}
>      (nil))
> (insn 25 23 26 2 (use (reg:SI 0 ax)) "pr93402.c":10:1 -1
>      (nil))
>-(insn 26 25 35 2 (use (reg:DI 1 dx)) "pr93402.c":10:1 -1
>+(insn 26 25 35 2 (use (plus:DI (reg:DI 1 dx [95])
>+            (reg:DI 5 di [92]))) "pr93402.c":10:1 -1
>      (nil))
>A USE insn is not a normal insn and verify_changes called from
>apply_change_group is happy about any changes into it.
>The following patch avoids this optimization if we were to change
>the USE operand (this routine only changes a reg into (plus reg reg2)).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
>later for release branches?

Ok. Looks like the same thing Richard fixed for autoinc. 

Richard. 

>2020-01-23  Jakub Jelinek  <jakub@redhat.com>
>
>	PR rtl-optimization/93402
>	* postreload.c (reload_combine_recognize_pattern): Don't try to adjust
>	USE insns.
>
>	* gcc.c-torture/execute/pr93402.c: New test.
>
>--- gcc/postreload.c.jj	2020-01-12 11:54:36.000000000 +0100
>+++ gcc/postreload.c	2020-01-23 17:23:25.359929516 +0100
>@@ -1078,6 +1078,10 @@ reload_combine_recognize_pattern (rtx_in
>       struct reg_use *use = reg_state[regno].reg_use + i;
>       if (GET_MODE (*use->usep) != mode)
> 	return false;
>+      /* Don't try to adjust (use (REGX)).  */
>+      if (GET_CODE (PATTERN (use->insn)) == USE
>+	  && &XEXP (PATTERN (use->insn), 0) == use->usep)
>+	return false;
>     }
> 
>   /* Look for (set (REGX) (CONST_INT))
>--- gcc/testsuite/gcc.c-torture/execute/pr93402.c.jj	2020-01-23
>17:25:46.496803852 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/pr93402.c	2020-01-23
>17:25:05.221425501 +0100
>@@ -0,0 +1,21 @@
>+/* PR rtl-optimization/93402 */
>+
>+struct S { unsigned int a; unsigned long long b; };
>+
>+__attribute__((noipa)) struct S
>+foo (unsigned long long x)
>+{
>+  struct S ret;
>+  ret.a = 0;
>+  ret.b = x * 11111111111ULL + 111111111111ULL;
>+  return ret;
>+}
>+
>+int
>+main ()
>+{
>+  struct S a = foo (1);
>+  if (a.a != 0 || a.b != 122222222222ULL)
>+    __builtin_abort ();
>+  return 0;
>+}
>
>	Jakub
Jeff Law Jan. 23, 2020, 7:10 p.m. UTC | #2
On Thu, 2020-01-23 at 20:05 +0100, Richard Biener wrote:
> On January 23, 2020 7:22:02 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
> > Hi!
> > 
> > The following testcase is miscompiled, because the postreload pass
> > changes:
> > -(insn 14 13 23 2 (parallel [
> > -            (set (reg:DI 1 dx [94])
> > -                (plus:DI (reg:DI 1 dx [95])
> > -                    (reg:DI 5 di [92])))
> > -            (clobber (reg:CC 17 flags))
> > -        ]) "pr93402.c":8:30 186 {*adddi_1}
> > -     (expr_list:REG_EQUAL (plus:DI (reg:DI 5 di [92])
> > -            (const_int 111111111111 [0x19debd01c7]))
> > -        (nil)))
> > -(insn 23 14 25 2 (set (reg:SI 0 ax)
> > +(insn 23 13 25 2 (set (reg:SI 0 ax)
> >         (const_int 0 [0])) "pr93402.c":10:1 67 {*movsi_internal}
> >      (nil))
> > (insn 25 23 26 2 (use (reg:SI 0 ax)) "pr93402.c":10:1 -1
> >      (nil))
> > -(insn 26 25 35 2 (use (reg:DI 1 dx)) "pr93402.c":10:1 -1
> > +(insn 26 25 35 2 (use (plus:DI (reg:DI 1 dx [95])
> > +            (reg:DI 5 di [92]))) "pr93402.c":10:1 -1
> >      (nil))
> > A USE insn is not a normal insn and verify_changes called from
> > apply_change_group is happy about any changes into it.
> > The following patch avoids this optimization if we were to change
> > the USE operand (this routine only changes a reg into (plus reg reg2)).
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> > later for release branches?
> 
> Ok. Looks like the same thing Richard fixed for autoinc. 
Yup.  I suppose we still need the naked clobber/use insns and that
there's more of these problems lurking in the weeks.

jeff
>

Patch
diff mbox series

--- gcc/postreload.c.jj	2020-01-12 11:54:36.000000000 +0100
+++ gcc/postreload.c	2020-01-23 17:23:25.359929516 +0100
@@ -1078,6 +1078,10 @@  reload_combine_recognize_pattern (rtx_in
       struct reg_use *use = reg_state[regno].reg_use + i;
       if (GET_MODE (*use->usep) != mode)
 	return false;
+      /* Don't try to adjust (use (REGX)).  */
+      if (GET_CODE (PATTERN (use->insn)) == USE
+	  && &XEXP (PATTERN (use->insn), 0) == use->usep)
+	return false;
     }
 
   /* Look for (set (REGX) (CONST_INT))
--- gcc/testsuite/gcc.c-torture/execute/pr93402.c.jj	2020-01-23 17:25:46.496803852 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr93402.c	2020-01-23 17:25:05.221425501 +0100
@@ -0,0 +1,21 @@ 
+/* PR rtl-optimization/93402 */
+
+struct S { unsigned int a; unsigned long long b; };
+
+__attribute__((noipa)) struct S
+foo (unsigned long long x)
+{
+  struct S ret;
+  ret.a = 0;
+  ret.b = x * 11111111111ULL + 111111111111ULL;
+  return ret;
+}
+
+int
+main ()
+{
+  struct S a = foo (1);
+  if (a.a != 0 || a.b != 122222222222ULL)
+    __builtin_abort ();
+  return 0;
+}