PR 49145: Another (zero_extend (const_int ...)) in combine

Richard Sandiford rdsandiford@googlemail.com
Sun May 29 09:30:00 GMT 2011


Three places in combine.c use SUBST to perform recursive replacements
on operands.  Two of them (subst and known_cond) handle the special
case of a zero_extend operand being optimised to a constant.  This patch
deals with the third, make_compound_operation.

In the PR, make_compound_operation is able to use nonzero_bits to
optimise a zero_extend argument to a constant.  The new rtx is then
(zero_extend:M (const_int N)), which is invalid.

Rather than add a copy of the known_cond code to make_compound_operation,
I decided to split the subst handling of this case out into a separate
function that could be used in all three places.

I've included the subreg handling as well as the zero_extend handling,
even though make_compound_operation already handles that case correctly.
However, I've cowardly not removed the known_cond subreg handling:

  else if (code == SUBREG)
    {
      enum machine_mode inner_mode = GET_MODE (SUBREG_REG (x));
      rtx new_rtx, r = known_cond (SUBREG_REG (x), cond, reg, val);

      if (SUBREG_REG (x) != r)
	{
	  /* We must simplify subreg here, before we lose track of the
	     original inner_mode.  */
	  new_rtx = simplify_subreg (GET_MODE (x), r,
				 inner_mode, SUBREG_BYTE (x));
	  if (new_rtx)
	    return new_rtx;
	  else
	    SUBST (SUBREG_REG (x), r);
	}

      return x;
    }

The new function would also handle the case described in the comment.
However, I was afraid that we might rely on simplify_subreg being
used for all simplified operands here, while also relying on it only
being used for constants in subst.  (This comes from a general
fear that combine is special in the way that it handles compound
operations.  Calling the generic simplification routines too often
could cause us to turn combine's preferred representation into
the representation normally used elsewhere.)

However, I'm happy to change it so that either the new function
calls simplify_subreg for everything, or that known_cond only
calls it for constants.  Let me know if you'd prefer one or
the other.  (TBH, I'd prefer the latter for consistency with
the unary case.)

The known_cond code has the comment:

  /* We don't have to handle SIGN_EXTEND here, because even in the
     case of replacing something with a modeless CONST_INT, a
     CONST_INT is already (supposed to be) a valid sign extension for
     its narrower mode, which implies it's already properly
     sign-extended for the wider mode.  Now, for ZERO_EXTEND, the
     story is different.  */

While that is true, I don't see any point in going out of our way
_not_ to handle sign_extend in the same way.  Or indeed all unary
operators.  Testing UNARY_P seems justified on the basis that
simplify_unary_operation requires the mode of the inner operand,
and that losing that mode without giving simplify_unary_operation
a chance could therefore lead to wrong results.  I'd rather not
see us hard-code the knowledge of which operators actually care.

I think it's justified for subst_operand not to handle binary
operators in the same way.  If we did want these functions to
simplify binary operators in some cases, I think that code is
more naturally done as an extension to:

  /* If this is a commutative operation, the changes to the operands
     may have made it noncanonical.  */
  if (COMMUTATIVE_ARITH_P (x)
      && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1)))
    {
      tem = XEXP (x, 0);
      SUBST (XEXP (x, 0), XEXP (x, 1));
      SUBST (XEXP (x, 1), tem);
    }

Binary and unary operations are different because the mode of
the inner operand is implied by the outer mode.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
on mips-linux-gnu.  OK to install?

Richard


gcc/
	PR rtl-optimization/49145
	* combine.c (subst_operand): New function, split out from...
	(subst): ...here and generalise ZERO_EXTEND handlig to all
	unary operations.
	(make_compound_operation): Call subst_operand.
	(known_cond): Likewise.  Do not use separate ZERO_EXTEND check.

gcc/testsuite/
	PR rtl-optimization/49145
	From Ryan Mansfield
	* gcc.c-torture/compile/pr49145.c: New test.

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2011-05-28 18:25:40.000000000 +0100
+++ gcc/combine.c	2011-05-28 18:27:31.000000000 +0100
@@ -4973,6 +4973,46 @@ find_split_point (rtx *loc, rtx insn, bo
     }
 }
 
+/* Replace operand I of *LOC with NEW_RTX.
+
+   In some cases, substituting a constant into an existing rtx can
+   lose vital information, such as the inner mode of a SUBREG or a
+   ZERO_EXTEND operation.  Return true if this substitution falls into
+   that category, and replace *LOC with a simplified form of the result.
+   Return false if *LOC points to the same rtx as before.  */
+
+static bool
+subst_operand (rtx *loc, int i, rtx new_rtx)
+{
+  rtx x, tem;
+
+  x = *loc;
+  if (CONST_INT_P (new_rtx) || GET_CODE (new_rtx) == CONST_DOUBLE)
+    {
+      if (GET_CODE (x) == SUBREG)
+	{
+	  tem = simplify_subreg (GET_MODE (x), new_rtx,
+				 GET_MODE (SUBREG_REG (x)),
+				 SUBREG_BYTE (x));
+	  if (!tem)
+	    tem = gen_rtx_CLOBBER (GET_MODE (x), const0_rtx);
+	}
+      else if (UNARY_P (x))
+	tem = simplify_unary_operation (GET_CODE (x), GET_MODE (x),
+					new_rtx, GET_MODE (XEXP (x, 0)));
+      else
+	tem = NULL_RTX;
+
+      if (tem)
+	{
+	  *loc = tem;
+	  return true;
+	}
+    }
+  SUBST (XEXP (x, i), new_rtx);
+  return false;
+}
+
 /* Throughout X, replace FROM with TO, and return the result.
    The result is TO if X is FROM;
    otherwise the result is X, but its contents may have been modified.
@@ -5210,27 +5250,8 @@ #define COMBINE_RTX_EQUAL_P(X,Y)			\
 	      if (GET_CODE (new_rtx) == CLOBBER && XEXP (new_rtx, 0) == const0_rtx)
 		return new_rtx;
 
-	      if (GET_CODE (x) == SUBREG
-		  && (CONST_INT_P (new_rtx)
-		      || GET_CODE (new_rtx) == CONST_DOUBLE))
-		{
-		  enum machine_mode mode = GET_MODE (x);
-
-		  x = simplify_subreg (GET_MODE (x), new_rtx,
-				       GET_MODE (SUBREG_REG (x)),
-				       SUBREG_BYTE (x));
-		  if (! x)
-		    x = gen_rtx_CLOBBER (mode, const0_rtx);
-		}
-	      else if (CONST_INT_P (new_rtx)
-		       && GET_CODE (x) == ZERO_EXTEND)
-		{
-		  x = simplify_unary_operation (ZERO_EXTEND, GET_MODE (x),
-						new_rtx, GET_MODE (XEXP (x, 0)));
-		  gcc_assert (x);
-		}
-	      else
-		SUBST (XEXP (x, i), new_rtx);
+	      if (subst_operand (&x, i, new_rtx))
+		break;
 	    }
 	}
     }
@@ -7887,7 +7908,8 @@ make_compound_operation (rtx x, enum rtx
     if (fmt[i] == 'e')
       {
 	new_rtx = make_compound_operation (XEXP (x, i), next_code);
-	SUBST (XEXP (x, i), new_rtx);
+	if (subst_operand (&x, i, new_rtx))
+	  break;
       }
     else if (fmt[i] == 'E')
       for (j = 0; j < XVECLEN (x, i); j++)
@@ -8942,37 +8964,15 @@ known_cond (rtx x, enum rtx_code cond, r
 
       return x;
     }
-  /* We don't have to handle SIGN_EXTEND here, because even in the
-     case of replacing something with a modeless CONST_INT, a
-     CONST_INT is already (supposed to be) a valid sign extension for
-     its narrower mode, which implies it's already properly
-     sign-extended for the wider mode.  Now, for ZERO_EXTEND, the
-     story is different.  */
-  else if (code == ZERO_EXTEND)
-    {
-      enum machine_mode inner_mode = GET_MODE (XEXP (x, 0));
-      rtx new_rtx, r = known_cond (XEXP (x, 0), cond, reg, val);
-
-      if (XEXP (x, 0) != r)
-	{
-	  /* We must simplify the zero_extend here, before we lose
-	     track of the original inner_mode.  */
-	  new_rtx = simplify_unary_operation (ZERO_EXTEND, GET_MODE (x),
-					  r, inner_mode);
-	  if (new_rtx)
-	    return new_rtx;
-	  else
-	    SUBST (XEXP (x, 0), r);
-	}
-
-      return x;
-    }
 
   fmt = GET_RTX_FORMAT (code);
   for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
     {
       if (fmt[i] == 'e')
-	SUBST (XEXP (x, i), known_cond (XEXP (x, i), cond, reg, val));
+	{
+	  if (subst_operand (&x, i, known_cond (XEXP (x, i), cond, reg, val)))
+	    break;
+	}
       else if (fmt[i] == 'E')
 	for (j = XVECLEN (x, i) - 1; j >= 0; j--)
 	  SUBST (XVECEXP (x, i, j), known_cond (XVECEXP (x, i, j),
Index: gcc/testsuite/gcc.c-torture/compile/pr49145.c
===================================================================
--- /dev/null	2011-05-28 09:03:52.070295797 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr49145.c	2011-05-28 18:27:31.000000000 +0100
@@ -0,0 +1,30 @@
+static int
+func1 (int a, int b)
+{
+  return b ? a : a / b;
+}
+
+static unsigned char
+func2 (unsigned char a, int b)
+{
+  return b ? a : b;
+}
+
+int i;
+
+void
+func3 (const int arg)
+{
+  for (i = 0; i != 10; i = foo ())
+    {
+      if (!arg)
+	{
+	  int j;
+	  for (j = 0; j < 5; j += 1)
+	    {
+	      int *ptr;
+	      *ptr = func2 (func1 (arg, *ptr), foo (arg));
+	    }
+	}
+    }
+}



More information about the Gcc-patches mailing list