diff mbox

Fix up cprop to canonicalize PLUS/MINUS with 2 CONSTANT_P arguments (PR rtl-optimization/57915)

Message ID 20140201095831.GB892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 1, 2014, 9:58 a.m. UTC 
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.



	Jakub

Comments

Jeff Law Feb. 3, 2014, 10:11 p.m. UTC | #1 
On 02/01/14 02:58, Jakub Jelinek wrote:
>> 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.
True.  But is that the only place that creates the expressions?  I'd 
certainly agree that PLUS is by far the most important.  I've never the 
other forms, but those are more likely an artifact of the ports I've 
worked on through the years.




>
> 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.
Some is certainly historical.  Even just unifying the simplifications in 
combine and simplify_rtx would be a big (but useful) change.  Agreed 
it's not stage4 material.


>
> 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.
Looks good to me.

Jeff
diff mbox

Patch

--- 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);
+    }
+}