Re: [PATCH][combine][1/2] Try to simplify before substituting

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

On 16/07/15 19:28, Segher Boessenkool wrote:
> On Thu, Jul 16, 2015 at 07:17:54PM +0100, Kyrill Tkachov wrote:
> > > If you always want to simplify first, does it work to move this whole big
> > > block behind the simplify just following it?  Or do you want to simplify
> > > after the transform as well?
> > You mean move this hunk outside the "if (BINARY_P (x)...)" block it's in?
> > I think it would work, but I'm not sure if it would affect other cases.
> > I was also conscious that simplify_rtx might not be a cheap function to call
> > so frequently (or is it? I didn't profile it), so I tried to avoid calling
> > it unless I need for the transformation in question here.
> I mean move the whole "if (BINARY_P ..." block to after the existing
> simplify calls, to just before the "First see if we can apply" comment,
> and not do a new simplify_rtx call at all.  Does that work?

Yes, and here's the patch.
It just moves the simplification block.
The effect on codegen in SPEC2006 on aarch64 looks sane in the same
way as the original patch I posted (i.e. many redundant zero_extends eliminated)
and together with patch 2/2 this helps in the -abs testcase.

I'm bootstrapping this on aarch64, arm and x86.
Any other testing would be appreciated.

Is this version ok if testing comes clean?

Thanks,
Kyrill

2015-07-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    * combine.c (combine_simplify_rtx): Move simplification step
    before various transformations/substitutions.


commit d7c7cd4acad9a34b3e991fd589125cc165f25c1f
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Thu Jul 16 19:42:58 2015 +0100

    [combine] Alternative approach

diff --git a/gcc/combine.c b/gcc/combine.c
index 574f874..2f806ab 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -5489,6 +5489,51 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest,
       SUBST (XEXP (x, 1), temp);
     }
 
+  /* Try to fold this expression in case we have constants that weren't
+     present before.  */
+  temp = 0;
+  switch (GET_RTX_CLASS (code))
+    {
+    case RTX_UNARY:
+      if (op0_mode == VOIDmode)
+	op0_mode = GET_MODE (XEXP (x, 0));
+      temp = simplify_unary_operation (code, mode, XEXP (x, 0), op0_mode);
+      break;
+    case RTX_COMPARE:
+    case RTX_COMM_COMPARE:
+      {
+	machine_mode cmp_mode = GET_MODE (XEXP (x, 0));
+	if (cmp_mode == VOIDmode)
+	  {
+	    cmp_mode = GET_MODE (XEXP (x, 1));
+	    if (cmp_mode == VOIDmode)
+	      cmp_mode = op0_mode;
+	  }
+	temp = simplify_relational_operation (code, mode, cmp_mode,
+					      XEXP (x, 0), XEXP (x, 1));
+      }
+      break;
+    case RTX_COMM_ARITH:
+    case RTX_BIN_ARITH:
+      temp = simplify_binary_operation (code, mode, XEXP (x, 0), XEXP (x, 1));
+      break;
+    case RTX_BITFIELD_OPS:
+    case RTX_TERNARY:
+      temp = simplify_ternary_operation (code, mode, op0_mode, XEXP (x, 0),
+					 XEXP (x, 1), XEXP (x, 2));
+      break;
+    default:
+      break;
+    }
+
+  if (temp)
+    {
+      x = temp;
+      code = GET_CODE (temp);
+      op0_mode = VOIDmode;
+      mode = GET_MODE (temp);
+    }
+
   /* If this is a simple operation applied to an IF_THEN_ELSE, try
      applying it to the arms of the IF_THEN_ELSE.  This often simplifies
      things.  Check for cases where both arms are testing the same
@@ -5588,51 +5633,6 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest,
 	}
     }
 
-  /* Try to fold this expression in case we have constants that weren't
-     present before.  */
-  temp = 0;
-  switch (GET_RTX_CLASS (code))
-    {
-    case RTX_UNARY:
-      if (op0_mode == VOIDmode)
-	op0_mode = GET_MODE (XEXP (x, 0));
-      temp = simplify_unary_operation (code, mode, XEXP (x, 0), op0_mode);
-      break;
-    case RTX_COMPARE:
-    case RTX_COMM_COMPARE:
-      {
-	machine_mode cmp_mode = GET_MODE (XEXP (x, 0));
-	if (cmp_mode == VOIDmode)
-	  {
-	    cmp_mode = GET_MODE (XEXP (x, 1));
-	    if (cmp_mode == VOIDmode)
-	      cmp_mode = op0_mode;
-	  }
-	temp = simplify_relational_operation (code, mode, cmp_mode,
-					      XEXP (x, 0), XEXP (x, 1));
-      }
-      break;
-    case RTX_COMM_ARITH:
-    case RTX_BIN_ARITH:
-      temp = simplify_binary_operation (code, mode, XEXP (x, 0), XEXP (x, 1));
-      break;
-    case RTX_BITFIELD_OPS:
-    case RTX_TERNARY:
-      temp = simplify_ternary_operation (code, mode, op0_mode, XEXP (x, 0),
-					 XEXP (x, 1), XEXP (x, 2));
-      break;
-    default:
-      break;
-    }
-
-  if (temp)
-    {
-      x = temp;
-      code = GET_CODE (temp);
-      op0_mode = VOIDmode;
-      mode = GET_MODE (temp);
-    }
-
   /* First see if we can apply the inverse distributive law.  */
   if (code == PLUS || code == MINUS
       || code == AND || code == IOR || code == XOR)