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]

Re: [PATCH] Fix PR27116, rework negate_expr_p/negate_expr


On Fri, 9 Jun 2006, Richard Guenther wrote:

> On Fri, 9 Jun 2006, Richard Guenther wrote:
> 
> > 
> > This patch fixes PR27116 for real by rewoking negate_expr_p to avoid
> > introducing undefined overflow.  It splits a generic fold_negate_expr
> > from negate_expr that can be used in fold_unary, fixing a latent problem
> > with RSHIFT_EXPR folding which didn't get noticed because fold_unary
> > passed a tree with stripped types to negate_expr (this problem is latent
> > on the branches).  With this patch I also revert the folding parts of
> > the first fix for PR27116, as these are not necessary.
> > 
> > The patch is big enough as is, but as a followup I will try to remove
> > the second argument to fold_negate_expr and put the burden of stripping
> > type conversions and re-applying them afterwards to the caller as other
> > fold_* variants do.  But I think this is not appropriate for 4.2 and has
> > to wait for stage1.
> 
> The more I think about the type stripping in the original negate_expr and
> the way its now in fold_negate_expr the less I like it.  I'll rework the
> patch next week.

Like this one.  Bootstrapped and regtested on x86_64-unknown-linux-gnu.

Richard.


2006-06-09  Richard Guenther  <rguenther@suse.de>

	PR middle-end/27116
	* fold-const.c (negate_expr_p): Do not introduce undefined
	overflow in negating INTEGER_CSTs.
	(fold_negate_expr): Rename from negate_expr.  Revert last
	change for folding BIT_NOT_EXPR.  Change semantics to
	return NULL_TREE for non-simplified negations.  Do not
	strip type conversions and unify type handling.
	(negate_expr): New function, wrap around fold_negate_expr
	but ensure building a tree always.  Strip type conversions
	here, fold to result type.
	(fold_unary): Use fold_negate_expr for folding NEGATE_EXPR.

	* gcc.dg/pr15785-1.c: Revert last change.
	* gcc.dg/torture/pr27116-2.c: New testcase.


Index: testsuite/gcc.dg/pr15785-1.c
===================================================================
*** testsuite/gcc.dg/pr15785-1.c	(revision 114483)
--- testsuite/gcc.dg/pr15785-1.c	(working copy)
*************** void b (int x) {
*** 11,16 ****
--- 11,21 ----
  		link_error ();
  }
  
+ void c (int x) {
+ 	if (!(- (~x) - x))
+ 		link_error ();
+ }
+ 
  void d (int x) {
  	if (!(~ (-x) - x))
  		link_error ();
*************** void f (int x) {
*** 29,34 ****
--- 34,40 ----
  int main (int argc, char *argv[]) {
  	a(argc);
  	b(argc);
+ 	c(argc);
  	d(argc);
  	e(argc);
  	f(argc);
Index: testsuite/gcc.dg/torture/pr27116-2.c
===================================================================
*** testsuite/gcc.dg/torture/pr27116-2.c	(revision 0)
--- testsuite/gcc.dg/torture/pr27116-2.c	(revision 0)
***************
*** 0 ****
--- 1,13 ----
+ /* { dg-do run } */
+ 
+ extern void abort(void);
+ 
+ int main (void)
+ {
+     volatile long int n;
+     n = -2;
+ 
+     if ((-2147483647L - 1L) / (-n) != -1073741824L)
+ 	abort ();
+     return 0;
+ }

Index: fold-const.c
===================================================================
*** fold-const.c	(revision 114506)
--- fold-const.c	(working copy)
*************** may_negate_without_overflow_p (tree t)
*** 923,929 ****
  }
  
  /* Determine whether an expression T can be cheaply negated using
!    the function negate_expr.  */
  
  static bool
  negate_expr_p (tree t)
--- 923,929 ----
  }
  
  /* Determine whether an expression T can be cheaply negated using
!    the function negate_expr without introducing undefined overflow.  */
  
  static bool
  negate_expr_p (tree t)
*************** negate_expr_p (tree t)
*** 939,945 ****
    switch (TREE_CODE (t))
      {
      case INTEGER_CST:
!       if (TYPE_UNSIGNED (type) || ! flag_trapv)
  	return true;
  
        /* Check that -CST will not overflow type.  */
--- 939,946 ----
    switch (TREE_CODE (t))
      {
      case INTEGER_CST:
!       if (TYPE_UNSIGNED (type)
! 	  || (flag_wrapv && ! flag_trapv))
  	return true;
  
        /* Check that -CST will not overflow type.  */
*************** negate_expr_p (tree t)
*** 1030,1057 ****
    return false;
  }
  
! /* Given T, an expression, return the negation of T.  Allow for T to be
!    null, in which case return null.  */
  
  static tree
! negate_expr (tree t)
  {
!   tree type;
    tree tem;
  
-   if (t == 0)
-     return 0;
- 
-   type = TREE_TYPE (t);
-   STRIP_SIGN_NOPS (t);
- 
    switch (TREE_CODE (t))
      {
      /* Convert - (~A) to A + 1.  */
      case BIT_NOT_EXPR:
!       if (INTEGRAL_TYPE_P (type)
!       	  && (TYPE_UNSIGNED (type)
! 	      || (flag_wrapv && !flag_trapv)))
          return fold_build2 (PLUS_EXPR, type, TREE_OPERAND (t, 0),
                              build_int_cst (type, 1));
        break;
--- 1031,1052 ----
    return false;
  }
  
! /* Given T, an expression, return a folded tree for -T or NULL_TREE, if no
!    simplification is possible.
!    If negate_expr_p would return true for T, NULL_TREE will never be
!    returned.  */
  
  static tree
! fold_negate_expr (tree t)
  {
!   tree type = TREE_TYPE (t);
    tree tem;
  
    switch (TREE_CODE (t))
      {
      /* Convert - (~A) to A + 1.  */
      case BIT_NOT_EXPR:
!       if (INTEGRAL_TYPE_P (type))
          return fold_build2 (PLUS_EXPR, type, TREE_OPERAND (t, 0),
                              build_int_cst (type, 1));
        break;
*************** negate_expr (tree t)
*** 1068,1074 ****
        tem = fold_negate_const (t, type);
        /* Two's complement FP formats, such as c4x, may overflow.  */
        if (! TREE_OVERFLOW (tem) || ! flag_trapping_math)
! 	return fold_convert (type, tem);
        break;
  
      case COMPLEX_CST:
--- 1063,1069 ----
        tem = fold_negate_const (t, type);
        /* Two's complement FP formats, such as c4x, may overflow.  */
        if (! TREE_OVERFLOW (tem) || ! flag_trapping_math)
! 	return tem;
        break;
  
      case COMPLEX_CST:
*************** negate_expr (tree t)
*** 1085,1091 ****
        break;
  
      case NEGATE_EXPR:
!       return fold_convert (type, TREE_OPERAND (t, 0));
  
      case PLUS_EXPR:
        if (! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
--- 1080,1086 ----
        break;
  
      case NEGATE_EXPR:
!       return TREE_OPERAND (t, 0);
  
      case PLUS_EXPR:
        if (! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
*************** negate_expr (tree t)
*** 1096,1113 ****
  				     TREE_OPERAND (t, 1)))
  	    {
  	      tem = negate_expr (TREE_OPERAND (t, 1));
! 	      tem = fold_build2 (MINUS_EXPR, TREE_TYPE (t),
! 				 tem, TREE_OPERAND (t, 0));
! 	      return fold_convert (type, tem);
  	    }
  
  	  /* -(A + B) -> (-A) - B.  */
  	  if (negate_expr_p (TREE_OPERAND (t, 0)))
  	    {
  	      tem = negate_expr (TREE_OPERAND (t, 0));
! 	      tem = fold_build2 (MINUS_EXPR, TREE_TYPE (t),
! 				 tem, TREE_OPERAND (t, 1));
! 	      return fold_convert (type, tem);
  	    }
  	}
        break;
--- 1091,1106 ----
  				     TREE_OPERAND (t, 1)))
  	    {
  	      tem = negate_expr (TREE_OPERAND (t, 1));
! 	      return fold_build2 (MINUS_EXPR, type,
! 				  tem, TREE_OPERAND (t, 0));
  	    }
  
  	  /* -(A + B) -> (-A) - B.  */
  	  if (negate_expr_p (TREE_OPERAND (t, 0)))
  	    {
  	      tem = negate_expr (TREE_OPERAND (t, 0));
! 	      return fold_build2 (MINUS_EXPR, type,
! 				  tem, TREE_OPERAND (t, 1));
  	    }
  	}
        break;
*************** negate_expr (tree t)
*** 1116,1148 ****
        /* - (A - B) -> B - A  */
        if ((! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
  	  && reorder_operands_p (TREE_OPERAND (t, 0), TREE_OPERAND (t, 1)))
! 	return fold_convert (type,
! 			     fold_build2 (MINUS_EXPR, TREE_TYPE (t),
! 					  TREE_OPERAND (t, 1),
! 					  TREE_OPERAND (t, 0)));
        break;
  
      case MULT_EXPR:
!       if (TYPE_UNSIGNED (TREE_TYPE (t)))
          break;
  
        /* Fall through.  */
  
      case RDIV_EXPR:
!       if (! HONOR_SIGN_DEPENDENT_ROUNDING (TYPE_MODE (TREE_TYPE (t))))
  	{
  	  tem = TREE_OPERAND (t, 1);
  	  if (negate_expr_p (tem))
! 	    return fold_convert (type,
! 				 fold_build2 (TREE_CODE (t), TREE_TYPE (t),
! 					      TREE_OPERAND (t, 0),
! 					      negate_expr (tem)));
  	  tem = TREE_OPERAND (t, 0);
  	  if (negate_expr_p (tem))
! 	    return fold_convert (type,
! 				 fold_build2 (TREE_CODE (t), TREE_TYPE (t),
! 					      negate_expr (tem),
! 					      TREE_OPERAND (t, 1)));
  	}
        break;
  
--- 1109,1135 ----
        /* - (A - B) -> B - A  */
        if ((! FLOAT_TYPE_P (type) || flag_unsafe_math_optimizations)
  	  && reorder_operands_p (TREE_OPERAND (t, 0), TREE_OPERAND (t, 1)))
! 	return fold_build2 (MINUS_EXPR, type,
! 			    TREE_OPERAND (t, 1), TREE_OPERAND (t, 0));
        break;
  
      case MULT_EXPR:
!       if (TYPE_UNSIGNED (type))
          break;
  
        /* Fall through.  */
  
      case RDIV_EXPR:
!       if (! HONOR_SIGN_DEPENDENT_ROUNDING (TYPE_MODE (type)))
  	{
  	  tem = TREE_OPERAND (t, 1);
  	  if (negate_expr_p (tem))
! 	    return fold_build2 (TREE_CODE (t), type,
! 				TREE_OPERAND (t, 0), negate_expr (tem));
  	  tem = TREE_OPERAND (t, 0);
  	  if (negate_expr_p (tem))
! 	    return fold_build2 (TREE_CODE (t), type,
! 				negate_expr (tem), TREE_OPERAND (t, 1));
  	}
        break;
  
*************** negate_expr (tree t)
*** 1151,1170 ****
      case FLOOR_DIV_EXPR:
      case CEIL_DIV_EXPR:
      case EXACT_DIV_EXPR:
!       if (!TYPE_UNSIGNED (TREE_TYPE (t)) && !flag_wrapv)
          {
            tem = TREE_OPERAND (t, 1);
            if (negate_expr_p (tem))
!             return fold_convert (type,
!                                  fold_build2 (TREE_CODE (t), TREE_TYPE (t),
!                                               TREE_OPERAND (t, 0),
!                                               negate_expr (tem)));
            tem = TREE_OPERAND (t, 0);
            if (negate_expr_p (tem))
!             return fold_convert (type,
!                                  fold_build2 (TREE_CODE (t), TREE_TYPE (t),
!                                               negate_expr (tem),
!                                               TREE_OPERAND (t, 1)));
          }
        break;
  
--- 1138,1153 ----
      case FLOOR_DIV_EXPR:
      case CEIL_DIV_EXPR:
      case EXACT_DIV_EXPR:
!       if (!TYPE_UNSIGNED (type) && !flag_wrapv)
          {
            tem = TREE_OPERAND (t, 1);
            if (negate_expr_p (tem))
!             return fold_build2 (TREE_CODE (t), type,
! 				TREE_OPERAND (t, 0), negate_expr (tem));
            tem = TREE_OPERAND (t, 0);
            if (negate_expr_p (tem))
!             return fold_build2 (TREE_CODE (t), type,
! 				negate_expr (tem), TREE_OPERAND (t, 1));
          }
        break;
  
*************** negate_expr (tree t)
*** 1174,1180 ****
  	{
  	  tem = strip_float_extensions (t);
  	  if (tem != t && negate_expr_p (tem))
! 	    return fold_convert (type, negate_expr (tem));
  	}
        break;
  
--- 1157,1163 ----
  	{
  	  tem = strip_float_extensions (t);
  	  if (tem != t && negate_expr_p (tem))
! 	    return negate_expr (tem);
  	}
        break;
  
*************** negate_expr (tree t)
*** 1215,1221 ****
        break;
      }
  
!   tem = fold_build1 (NEGATE_EXPR, TREE_TYPE (t), t);
    return fold_convert (type, tem);
  }
  
--- 1198,1224 ----
        break;
      }
  
!   return NULL_TREE;
! }
! 
! /* Like fold_negate_expr, but return a NEGATE_EXPR tree, if T can not be
!    negated in a simpler way.  Also allow for T to be NULL_TREE, in which case
!    return NULL_TREE. */
! 
! static tree
! negate_expr (tree t)
! {
!   tree type, tem;
! 
!   if (t == NULL_TREE)
!     return NULL_TREE;
! 
!   type = TREE_TYPE (t);
!   STRIP_SIGN_NOPS (t);
! 
!   tem = fold_negate_expr (t);
!   if (!tem)
!     tem = build1 (NEGATE_EXPR, TREE_TYPE (t), t);
    return fold_convert (type, tem);
  }
  
*************** fold_unary (enum tree_code code, tree ty
*** 7521,7528 ****
        return fold_view_convert_expr (type, op0);
  
      case NEGATE_EXPR:
!       if (negate_expr_p (arg0))
! 	return fold_convert (type, negate_expr (arg0));
        return NULL_TREE;
  
      case ABS_EXPR:
--- 7524,7532 ----
        return fold_view_convert_expr (type, op0);
  
      case NEGATE_EXPR:
!       tem = fold_negate_expr (arg0);
!       if (tem)
! 	return fold_convert (type, tem);
        return NULL_TREE;
  
      case ABS_EXPR:


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