This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Constant folding clean-ups


The following patch contains two clean-ups of GCC's constant folding.

The first is to remove most of the places where fold modifies the
current tree in-place, replacing TREE_SET_CODE with the prefered
recursive invocation as "fold (build (code, type, arg0, arg1))".
This clean-up avoids ordering problems with some transformations and
reduces potential problems where the caller assumes the original
argument to fold is immutable.

The second clean-up that I've been meaning to do for a while is to
factor the logic that determines whether we should swap operands into
a single function.  This code was previously duplicated in three or
four places and none of them canonicalized complex constants last!
The motivation for doing this in the same patch is that it prevents
the problem of unbounded recursion where we'd convert "(op A B)" into
"(op B A)" which we'd then try folding back to the original.  The new
tree_swap_operands_p not only commonizes the relevant code but also
specifies the "canonical" order so that we don't permute operands
needlessly.  Ideally, fold should be idempotent, such that fold is
a no-op on a previously folded tree, i.e. fold (fold (x)) == fold (x).

Dale, let me know if this resolves your "(uint-- > 0)" doloop issue?


The following patch has been bootstrapped on i686-pc-linux-gnu with a
full "make bootstrap", all languages except treelang, and regression
tested with a top-level "make -k check" with no new failures.

Ok for mainline?

2003-09-13  Roger Sayle  <roger@eyesopen.com>

	* fold-const.c (tree_swap_operands_p): New function to determine
	the prefered ordering of operands.
	(fold): Numerous clean-ups.  Use tree_swap_operands_p when swapping
	operands to commutative, comparison or ternary operators.  Replace
	uses of TREE_SET_CODE with recursive call to fold.  Remove duplicate
	transformation of A ? B : C into !A ? C : B.


Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.302
diff -c -3 -p -r1.302 fold-const.c
*** fold-const.c	9 Sep 2003 22:10:30 -0000	1.302
--- fold-const.c	13 Sep 2003 14:50:06 -0000
*************** static bool fold_real_zero_addition_p (t
*** 108,113 ****
--- 108,114 ----
  static tree fold_mathfn_compare (enum built_in_function, enum tree_code,
  				 tree, tree, tree);
  static tree fold_inf_compare (enum tree_code, tree, tree, tree);
+ static bool tree_swap_operands_p (tree, tree);

  /* The following constants represent a bit based encoding of GCC's
     comparison operators.  This encoding simplifies transformations
*************** fold_single_bit_test (enum tree_code cod
*** 4976,4981 ****
--- 4977,5025 ----
    return NULL_TREE;
  }

+ /* Test whether it is preferable two swap two operands, ARG0 and
+    ARG1, for example because ARG0 is an integer constant and ARG1
+    isn't.  */
+
+ static bool
+ tree_swap_operands_p (tree arg0, tree arg1)
+ {
+   STRIP_SIGN_NOPS (arg0);
+   STRIP_SIGN_NOPS (arg1);
+
+   if (TREE_CODE (arg1) == INTEGER_CST)
+     return 0;
+   if (TREE_CODE (arg0) == INTEGER_CST)
+     return 1;
+
+   if (TREE_CODE (arg1) == REAL_CST)
+     return 0;
+   if (TREE_CODE (arg0) == REAL_CST)
+     return 1;
+
+   if (TREE_CODE (arg1) == COMPLEX_CST)
+     return 0;
+   if (TREE_CODE (arg0) == COMPLEX_CST)
+     return 1;
+
+   if (TREE_CONSTANT (arg1))
+     return 0;
+   if (TREE_CONSTANT (arg0))
+     return 1;
+
+   if (DECL_P (arg1))
+     return 0;
+   if (DECL_P (arg0))
+     return 1;
+
+   if (TREE_CODE (arg1) == SAVE_EXPR)
+     return 0;
+   if (TREE_CODE (arg0) == SAVE_EXPR)
+     return 1;
+
+   return 0;
+ }
+
  /* Perform constant folding and related simplification of EXPR.
     The related simplifications include x*1 => x, x*0 => 0, etc.,
     and application of the associative law.
*************** fold (tree expr)
*** 5035,5042 ****
  	subop = arg0;

        if (subop != 0 && TREE_CODE (subop) != INTEGER_CST
! 	  && TREE_CODE (subop) != REAL_CST
! 	  )
  	/* Note that TREE_CONSTANT isn't enough:
  	   static var addresses are constant but we can't
  	   do arithmetic on them.  */
--- 5079,5085 ----
  	subop = arg0;

        if (subop != 0 && TREE_CODE (subop) != INTEGER_CST
! 	  && TREE_CODE (subop) != REAL_CST)
  	/* Note that TREE_CONSTANT isn't enough:
  	   static var addresses are constant but we can't
  	   do arithmetic on them.  */
*************** fold (tree expr)
*** 5088,5103 ****
    if ((code == PLUS_EXPR || code == MULT_EXPR || code == MIN_EXPR
         || code == MAX_EXPR || code == BIT_IOR_EXPR || code == BIT_XOR_EXPR
         || code == BIT_AND_EXPR)
!       && ((TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) != INTEGER_CST)
! 	  || (TREE_CODE (arg0) == REAL_CST && TREE_CODE (arg1) != REAL_CST)))
!     {
!       tem = arg0; arg0 = arg1; arg1 = tem;
!
!       if (t == orig_t)
! 	t = copy_node (t);
!       TREE_OPERAND (t, 0) = arg0;
!       TREE_OPERAND (t, 1) = arg1;
!     }

    /* Now WINS is set as described above,
       ARG0 is the first operand of EXPR,
--- 5131,5138 ----
    if ((code == PLUS_EXPR || code == MULT_EXPR || code == MIN_EXPR
         || code == MAX_EXPR || code == BIT_IOR_EXPR || code == BIT_XOR_EXPR
         || code == BIT_AND_EXPR)
!       && tree_swap_operands_p (arg0, arg1))
!     return fold (build (code, type, arg1, arg0));

    /* Now WINS is set as described above,
       ARG0 is the first operand of EXPR,
*************** fold (tree expr)
*** 6645,6662 ****
  	 RROTATE_EXPR by a new constant.  */
        if (code == LROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST)
  	{
! 	  if (t == orig_t)
! 	    t = copy_node (t);
! 	  TREE_SET_CODE (t, RROTATE_EXPR);
! 	  code = RROTATE_EXPR;
! 	  TREE_OPERAND (t, 1) = arg1
! 	    = const_binop
! 	      (MINUS_EXPR,
! 	       convert (TREE_TYPE (arg1),
! 			build_int_2 (GET_MODE_BITSIZE (TYPE_MODE (type)), 0)),
! 	       arg1, 0);
! 	  if (tree_int_cst_sgn (arg1) < 0)
! 	    return t;
  	}

        /* If we have a rotate of a bit operation with the rotate count and
--- 6680,6689 ----
  	 RROTATE_EXPR by a new constant.  */
        if (code == LROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST)
  	{
! 	  tree tem = build_int_2 (GET_MODE_BITSIZE (TYPE_MODE (type)), 0);
! 	  tem = convert (TREE_TYPE (arg1), tem);
! 	  tem = const_binop (MINUS_EXPR, tem, arg1, 0);
! 	  return fold (build (RROTATE_EXPR, type, arg0, tem));
  	}

        /* If we have a rotate of a bit operation with the rotate count and
*************** fold (tree expr)
*** 6853,6872 ****
      case LE_EXPR:
      case GE_EXPR:
        /* If one arg is a real or integer constant, put it last.  */
!       if ((TREE_CODE (arg0) == INTEGER_CST
! 	   && TREE_CODE (arg1) != INTEGER_CST)
! 	  || (TREE_CODE (arg0) == REAL_CST
! 	      && TREE_CODE (arg0) != REAL_CST))
! 	{
! 	  if (t == orig_t)
! 	    t = copy_node (t);
! 	  TREE_OPERAND (t, 0) = arg1;
! 	  TREE_OPERAND (t, 1) = arg0;
! 	  arg0 = TREE_OPERAND (t, 0);
! 	  arg1 = TREE_OPERAND (t, 1);
! 	  code = swap_tree_comparison (code);
! 	  TREE_SET_CODE (t, code);
! 	}

        if (FLOAT_TYPE_P (TREE_TYPE (arg0)))
  	{
--- 6880,6887 ----
      case LE_EXPR:
      case GE_EXPR:
        /* If one arg is a real or integer constant, put it last.  */
!       if (tree_swap_operands_p (arg0, arg1))
! 	return fold (build (swap_tree_comparison (code), type, arg1, arg0));

        if (FLOAT_TYPE_P (TREE_TYPE (arg0)))
  	{
*************** fold (tree expr)
*** 7123,7138 ****
  	  switch (code)
  	    {
  	    case GE_EXPR:
- 	      code = GT_EXPR;
  	      arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 	      t = build (code, type, TREE_OPERAND (t, 0), arg1);
! 	      break;

  	    case LT_EXPR:
- 	      code = LE_EXPR;
  	      arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 	      t = build (code, type, TREE_OPERAND (t, 0), arg1);
! 	      break;

  	    default:
  	      break;
--- 7138,7149 ----
  	  switch (code)
  	    {
  	    case GE_EXPR:
  	      arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 	      return fold (build (GT_EXPR, type, arg0, arg1));

  	    case LT_EXPR:
  	      arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 	      return fold (build (LE_EXPR, type, arg0, arg1));

  	    default:
  	      break;
*************** fold (tree expr)
*** 7175,7198 ****
  					   convert (type, integer_zero_node),
  					   arg0);
  		case GE_EXPR:
! 		  code = EQ_EXPR;
! 		  if (t == orig_t)
! 		    t = copy_node (t);
! 		  TREE_SET_CODE (t, EQ_EXPR);
! 		  break;
  		case LE_EXPR:
  		  return omit_one_operand (type,
  					   convert (type, integer_one_node),
  					   arg0);
  		case LT_EXPR:
! 		  code = NE_EXPR;
! 		  if (t == orig_t)
! 		    t = copy_node (t);
! 		  TREE_SET_CODE (t, NE_EXPR);
! 		  break;

  		/* The GE_EXPR and LT_EXPR cases above are not normally
! 		   reached because of  previous transformations.  */

  		default:
  		  break;
--- 7186,7202 ----
  					   convert (type, integer_zero_node),
  					   arg0);
  		case GE_EXPR:
! 		  return fold (build (EQ_EXPR, type, arg0, arg1));
!
  		case LE_EXPR:
  		  return omit_one_operand (type,
  					   convert (type, integer_one_node),
  					   arg0);
  		case LT_EXPR:
! 		  return fold (build (NE_EXPR, type, arg0, arg1));

  		/* The GE_EXPR and LT_EXPR cases above are not normally
! 		   reached because of previous transformations.  */

  		default:
  		  break;
*************** fold (tree expr)
*** 7202,7216 ****
  	      switch (code)
  		{
  		case GT_EXPR:
- 		  code = EQ_EXPR;
  		  arg1 = const_binop (PLUS_EXPR, arg1, integer_one_node, 0);
! 		  t = build (code, type, TREE_OPERAND (t, 0), arg1);
! 		  break;
  		case LE_EXPR:
- 		  code = NE_EXPR;
  		  arg1 = const_binop (PLUS_EXPR, arg1, integer_one_node, 0);
! 		  t = build (code, type, TREE_OPERAND (t, 0), arg1);
! 		  break;
  		default:
  		  break;
  		}
--- 7206,7216 ----
  	      switch (code)
  		{
  		case GT_EXPR:
  		  arg1 = const_binop (PLUS_EXPR, arg1, integer_one_node, 0);
! 		  return fold (build (EQ_EXPR, type, arg0, arg1));
  		case LE_EXPR:
  		  arg1 = const_binop (PLUS_EXPR, arg1, integer_one_node, 0);
! 		  return fold (build (NE_EXPR, type, arg0, arg1));
  		default:
  		  break;
  		}
*************** fold (tree expr)
*** 7223,7244 ****
  					   convert (type, integer_zero_node),
  					   arg0);
  		case LE_EXPR:
! 		  code = EQ_EXPR;
! 		  if (t == orig_t)
! 		    t = copy_node (t);
! 		  TREE_SET_CODE (t, EQ_EXPR);
! 		  break;

  		case GE_EXPR:
  		  return omit_one_operand (type,
  					   convert (type, integer_one_node),
  					   arg0);
  		case GT_EXPR:
! 		  code = NE_EXPR;
! 		  if (t == orig_t)
! 		    t = copy_node (t);
! 		  TREE_SET_CODE (t, NE_EXPR);
! 		  break;

  		default:
  		  break;
--- 7223,7236 ----
  					   convert (type, integer_zero_node),
  					   arg0);
  		case LE_EXPR:
! 		  return fold (build (EQ_EXPR, type, arg0, arg1));

  		case GE_EXPR:
  		  return omit_one_operand (type,
  					   convert (type, integer_one_node),
  					   arg0);
  		case GT_EXPR:
! 		  return fold (build (NE_EXPR, type, arg0, arg1));

  		default:
  		  break;
*************** fold (tree expr)
*** 7248,7262 ****
  	      switch (code)
  		{
  		case GE_EXPR:
- 		  code = NE_EXPR;
  		  arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 		  t = build (code, type, TREE_OPERAND (t, 0), arg1);
! 		  break;
  		case LT_EXPR:
- 		  code = EQ_EXPR;
  		  arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 		  t = build (code, type, TREE_OPERAND (t, 0), arg1);
! 		  break;
  		default:
  		  break;
  		}
--- 7240,7250 ----
  	      switch (code)
  		{
  		case GE_EXPR:
  		  arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 		  return fold (build (NE_EXPR, type, arg0, arg1));
  		case LT_EXPR:
  		  arg1 = const_binop (MINUS_EXPR, arg1, integer_one_node, 0);
! 		  return fold (build (EQ_EXPR, type, arg0, arg1));
  		default:
  		  break;
  		}
*************** fold (tree expr)
*** 7489,7504 ****
  	  switch (code)
  	    {
  	    case EQ_EXPR:
  	    case GE_EXPR:
  	    case LE_EXPR:
  	      if (! FLOAT_TYPE_P (TREE_TYPE (arg0))
  		  || ! HONOR_NANS (TYPE_MODE (TREE_TYPE (arg0))))
  		return constant_boolean_node (1, type);
! 	      code = EQ_EXPR;
! 	      if (t == orig_t)
! 		t = copy_node (t);
! 	      TREE_SET_CODE (t, code);
! 	      break;

  	    case NE_EXPR:
  	      /* For NE, we can only do this simplification if integer
--- 7477,7493 ----
  	  switch (code)
  	    {
  	    case EQ_EXPR:
+ 	      if (! FLOAT_TYPE_P (TREE_TYPE (arg0))
+ 		  || ! HONOR_NANS (TYPE_MODE (TREE_TYPE (arg0))))
+ 		return constant_boolean_node (1, type);
+ 	      break;
+
  	    case GE_EXPR:
  	    case LE_EXPR:
  	      if (! FLOAT_TYPE_P (TREE_TYPE (arg0))
  		  || ! HONOR_NANS (TYPE_MODE (TREE_TYPE (arg0))))
  		return constant_boolean_node (1, type);
! 	      return fold (build (EQ_EXPR, type, arg0, arg1));

  	    case NE_EXPR:
  	      /* For NE, we can only do this simplification if integer
*************** fold (tree expr)
*** 7779,7812 ****
        else if (operand_equal_p (arg1, TREE_OPERAND (expr, 2), 0))
  	return pedantic_omit_one_operand (type, arg1, arg0);

-       /* If the second operand is zero, invert the comparison and swap
- 	 the second and third operands.  Likewise if the second operand
- 	 is constant and the third is not or if the third operand is
- 	 equivalent to the first operand of the comparison.  */
-
-       if (integer_zerop (arg1)
- 	  || (TREE_CONSTANT (arg1) && ! TREE_CONSTANT (TREE_OPERAND (t, 2)))
- 	  || (TREE_CODE_CLASS (TREE_CODE (arg0)) == '<'
- 	      && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
- 						 TREE_OPERAND (t, 2),
- 						 TREE_OPERAND (arg0, 1))))
- 	{
- 	  /* See if this can be inverted.  If it can't, possibly because
- 	     it was a floating-point inequality comparison, don't do
- 	     anything.  */
- 	  tem = invert_truthvalue (arg0);
-
- 	  if (TREE_CODE (tem) != TRUTH_NOT_EXPR)
- 	    {
- 	      t = build (code, type, tem,
- 			 TREE_OPERAND (t, 2), TREE_OPERAND (t, 1));
- 	      arg0 = tem;
- 	      /* arg1 should be the first argument of the new T.  */
- 	      arg1 = TREE_OPERAND (t, 1);
- 	      STRIP_NOPS (arg1);
- 	    }
- 	}
-
        /* If we have A op B ? A : C, we may be able to convert this to a
  	 simpler expression, depending on the operation and the values
  	 of B and C.  Signed zeros prevent all of these transformations,
--- 7768,7773 ----
*************** fold (tree expr)
*** 7982,7990 ****
  	      case EQ_EXPR:
  		/* We can replace A with C1 in this case.  */
  		arg1 = convert (type, TREE_OPERAND (arg0, 1));
! 		t = build (code, type, TREE_OPERAND (t, 0), arg1,
! 			   TREE_OPERAND (t, 2));
! 		break;

  	      case LT_EXPR:
  		/* If C1 is C2 + 1, this is min(A, C2).  */
--- 7943,7950 ----
  	      case EQ_EXPR:
  		/* We can replace A with C1 in this case.  */
  		arg1 = convert (type, TREE_OPERAND (arg0, 1));
! 		return fold (build (code, type, TREE_OPERAND (t, 0), arg1,
! 				    TREE_OPERAND (t, 2)));

  	      case LT_EXPR:
  		/* If C1 is C2 + 1, this is min(A, C2).  */
*************** fold (tree expr)
*** 8034,8044 ****

        /* If the second operand is simpler than the third, swap them
  	 since that produces better jump optimization results.  */
!       if ((TREE_CONSTANT (arg1) || DECL_P (arg1)
! 	   || TREE_CODE (arg1) == SAVE_EXPR)
! 	  && ! (TREE_CONSTANT (TREE_OPERAND (t, 2))
! 		|| DECL_P (TREE_OPERAND (t, 2))
! 		|| TREE_CODE (TREE_OPERAND (t, 2)) == SAVE_EXPR))
  	{
  	  /* See if this can be inverted.  If it can't, possibly because
  	     it was a floating-point inequality comparison, don't do
--- 7994,8000 ----

        /* If the second operand is simpler than the third, swap them
  	 since that produces better jump optimization results.  */
!       if (tree_swap_operands_p (TREE_OPERAND (t, 1), TREE_OPERAND (t, 2)))
  	{
  	  /* See if this can be inverted.  If it can't, possibly because
  	     it was a floating-point inequality comparison, don't do
*************** fold (tree expr)
*** 8046,8059 ****
  	  tem = invert_truthvalue (arg0);

  	  if (TREE_CODE (tem) != TRUTH_NOT_EXPR)
! 	    {
! 	      t = build (code, type, tem,
! 			 TREE_OPERAND (t, 2), TREE_OPERAND (t, 1));
! 	      arg0 = tem;
! 	      /* arg1 should be the first argument of the new T.  */
! 	      arg1 = TREE_OPERAND (t, 1);
! 	      STRIP_NOPS (arg1);
! 	    }
  	}

        /* Convert A ? 1 : 0 to simply A.  */
--- 8002,8009 ----
  	  tem = invert_truthvalue (arg0);

  	  if (TREE_CODE (tem) != TRUTH_NOT_EXPR)
! 	    return fold (build (code, type, tem,
! 			 TREE_OPERAND (t, 2), TREE_OPERAND (t, 1)));
  	}

        /* Convert A ? 1 : 0 to simply A.  */

Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]