Re: [PATCH] Fix up cprop to canonicalize PLUS/MINUS with 2 CONSTANT_P arguments (PR rtl-optimization/57915)

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Jan 31, 2014 at 11:12:58PM -0700, Jeff Law wrote:
> On 01/31/14 22:52, Richard Biener wrote:
> >On February 1, 2014 12:56:45 AM GMT+01:00, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> >>On Fri, Jan 31, 2014 at 10:28 PM, Jakub Jelinek wrote:
> >>
> >>>Without this patch, cprop can propagate e.g. a SYMBOL_REF to former
> >>>(mem (plus (reg) (const_int)))
> >>>making it invalid RTL (as plus of CONSTANT_P arguments must be
> >>simplified
> >>>or surrounded by CONST).
> >>
> >>But isn't this true for all non-unary operators? Is covering MINUS and
> >>PLUS enough?
> >
> >AND should be handled as well.  I can't think of any others that might be used with symbol refs.  But yes, in principle you are right.
> I think AND is necessary for certain variants of the Alpha.  In
> theory a shift-add might be possible here, but I don't know if it
> happens in practice.

Well, from what I can read in simplify-rtx.c, it only adds CONST around
PLUS/MINUS, not around anything else.

Anyway, another variant would be (untested so far) say following patch.

BTW, I wonder why each pass rolls it's own kind of simplification, e.g.
fwprop has propagate_rtx_1, cprop this function, etc., while we have
simplify_replace_fn_rtx that would be used perhaps with some hook that will
handle special cases for each of the pass, but otherwise will take care of
simplifying everything that has actually changed.  But that is likely not
what we want to change in stage4.

2014-02-01  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/57915
	* recog.c (simplify_while_replacing): If all unary/binary/relational
	operation arguments are constant, attempt to simplify those.

	* gcc.target/i386/pr57915.c: New test.

--- gcc/recog.c.jj	2014-01-31 22:21:42.832750702 +0100
+++ gcc/recog.c	2014-02-01 10:47:42.505475411 +0100
@@ -565,7 +565,7 @@ simplify_while_replacing (rtx *loc, rtx
 {
   rtx x = *loc;
   enum rtx_code code = GET_CODE (x);
-  rtx new_rtx;
+  rtx new_rtx = NULL_RTX;
 
   if (SWAPPABLE_OPERANDS_P (x)
       && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1)))
@@ -579,6 +579,35 @@ simplify_while_replacing (rtx *loc, rtx
       code = GET_CODE (x);
     }
 
+  /* Canonicalize arithmetics with all constant operands.  */
+  switch (GET_RTX_CLASS (code))
+    {
+    case RTX_UNARY:
+      if (CONSTANT_P (XEXP (x, 0)))
+	new_rtx = simplify_unary_operation (code, GET_MODE (x), XEXP (x, 0),
+					    op0_mode);
+      break;
+    case RTX_COMM_ARITH:
+    case RTX_BIN_ARITH:
+      if (CONSTANT_P (XEXP (x, 0)) && CONSTANT_P (XEXP (x, 1)))
+	new_rtx = simplify_binary_operation (code, GET_MODE (x), XEXP (x, 0),
+					     XEXP (x, 1));
+      break;
+    case RTX_COMPARE:
+    case RTX_COMM_COMPARE:
+      if (CONSTANT_P (XEXP (x, 0)) && CONSTANT_P (XEXP (x, 1)))
+	new_rtx = simplify_relational_operation (code, GET_MODE (x), op0_mode,
+						 XEXP (x, 0), XEXP (x, 1));
+      break;
+    default:
+      break;
+    }
+  if (new_rtx)
+    {
+      validate_change (object, loc, new_rtx, 1);
+      return;
+    }
+
   switch (code)
     {
     case PLUS:
--- gcc/testsuite/gcc.target/i386/pr57915.c.jj	2014-02-01 10:38:18.450137656 +0100
+++ gcc/testsuite/gcc.target/i386/pr57915.c	2014-02-01 10:38:18.449137657 +0100
@@ -0,0 +1,33 @@
+/* PR rtl-optimization/57915 */
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+
+extern struct T { char a[8]; char b[16]; } t;
+int c;
+void foo (void);
+
+extern inline char *
+baz (char *x, const char *y)
+{
+  const char *e = y;
+  unsigned long f, g;
+  asm ("" : "+c" (f), "+D" (e) : "a" ('\0'), "X" (*e));
+  g = e - 1 - y;
+  __builtin_memcpy (x, y, g);
+  x[g] = '\0';
+  return x;
+}
+
+void
+bar (void)
+{
+  char d[16];
+  baz (d, t.b);
+
+  for (;;)
+    {
+      foo ();
+      if (c)
+	baz (d, t.b);
+    }
+}


	Jakub