On Mon, 9 Oct 2017, Richard Biener wrote:

On Sun, Oct 8, 2017 at 1:22 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
Hello,

this moves (and extends a bit) one more transformation from fold-const.c to
match.pd. The single_use restriction is necessary for consistency with the
existing X+CST1 CMP CST2 transformation (if we do only one of the 2
transformations, gcc.dg/tree-ssa/vrp54.c regresses because DOM fails to
simplify 2 conditions based on different variables). The relaxation of the
condition to simplify (T) X == (T) Y is an easier way to avoid regressing
gcc.dg/tree-ssa/foldaddr-1.c than adding plenty of convert? in the patterns,
although that may still prove necessary at some point. I left a few odd
float cases in fold-const.c for later.

+/* X + Y < Y is the same as X < 0 when there is no overflow.  */
+(for op  (lt le ge gt)
+     rop (gt ge le lt)
+ (simplify
+  (op (plus:c@2 @0 @1) @1)
+  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+       && (CONSTANT_CLASS_P (@0) || single_use (@2)))
+   (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
+ (simplify
+  (op @1 (plus:c@2 @0 @1))
+  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+       && (CONSTANT_CLASS_P (@0) || single_use (@2)))
+   (rop @0 { build_zero_cst (TREE_TYPE (@0)); }))))

any reason (op:c (plus... on the first of the two patterns wouldn't have catched
the second?  :c on comparison swaps the comparison code.

I had completely forgotten that you had added this cool feature.

+/* For equality, this is also true with wrapping overflow.  */
+(for op (eq ne)
+ (simplify
+  (op:c (plus:c@2 @0 @1) @1)
+  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+          || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
+       && (CONSTANT_CLASS_P (@0) || single_use (@2)))
+   (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
+ (simplify
+  (op:c (convert?@3 (pointer_plus@2 @0 @1)) (convert? @0))
+  (if (CONSTANT_CLASS_P (@1) || (single_use (@2) && single_use (@3)))
+   (op @1 { build_zero_cst (TREE_TYPE (@1)); }))))

for consistency can you add the convert?s to the integer variant as well?

The only relevant case I could think of is, for unsigned u, (unsigned)((int)u+1)==u. All the other cases seem to be handled some other way. I still wrote it to allow conversions all over the place, but that's uglier than the MINUS_EXPR transformation below where I didn't.

I'm not sure you'll see the convers like you write them (with matching @0).

int f(char*p){
  char*q=p+5;
  return (long)q==(long)p;
}

Eventually on GENERIC when we still have pointer conversions around?

For generic we might want @@0 or pointers sometimes fail to match. At least in the POINTER_DIFF_EXPR experiment I had to do that in some transformations for constexpr.

You are allowing arbitrary conversions here - is that desired?  Isn't
a tree_nop_conversion_p missing?

I had the impression that casts from a pointer to whatever were always NOP, for instance when I try to cast char* to short, gcc produces (short)(long)p. But reading the comment in verify_gimple_assign_unary it is more complicated, so yes I should add a constraint.

        /* Allow conversions from pointer type to integral type only if
           there is no sign or zero extension involved.
           For targets were the precision of ptrofftype doesn't match that
           of pointers we need to allow arbitrary conversions to ptrofftype.  */
        if ((POINTER_TYPE_P (lhs_type)
             && INTEGRAL_TYPE_P (rhs1_type))
            || (POINTER_TYPE_P (rhs1_type)
                && INTEGRAL_TYPE_P (lhs_type)
                && (TYPE_PRECISION (rhs1_type) >= TYPE_PRECISION (lhs_type)
                    || ptrofftype_p (sizetype))))
          return false;

The code "|| ptrofftype_p (sizetype)" doesn't seem to match the comment :-(


Here is a new version of the patch, same changelog/testing.

--
Marc Glisse
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 253558)
+++ gcc/fold-const.c	(working copy)
@@ -10483,54 +10483,20 @@ fold_binary_loc (location_t loc,
           && code == EQ_EXPR)
         return fold_convert_loc (loc, type,
 				 fold_build1_loc (loc, TRUTH_NOT_EXPR,
 						  TREE_TYPE (arg0), arg0));
 
       /* !exp != 0 becomes !exp */
       if (TREE_CODE (arg0) == TRUTH_NOT_EXPR && integer_zerop (arg1)
 	  && code == NE_EXPR)
         return non_lvalue_loc (loc, fold_convert_loc (loc, type, arg0));
 
-      /* Transform comparisons of the form X +- Y CMP X to Y CMP 0.  */
-      if ((TREE_CODE (arg0) == PLUS_EXPR
-	   || TREE_CODE (arg0) == POINTER_PLUS_EXPR
-	   || TREE_CODE (arg0) == MINUS_EXPR)
-	  && operand_equal_p (tree_strip_nop_conversions (TREE_OPERAND (arg0,
-									0)),
-			      arg1, 0)
-	  && (INTEGRAL_TYPE_P (TREE_TYPE (arg0))
-	      || POINTER_TYPE_P (TREE_TYPE (arg0))))
-	{
-	  tree val = TREE_OPERAND (arg0, 1);
-	  val = fold_build2_loc (loc, code, type, val,
-				 build_int_cst (TREE_TYPE (val), 0));
-	  return omit_two_operands_loc (loc, type, val,
-					TREE_OPERAND (arg0, 0), arg1);
-	}
-
-      /* Transform comparisons of the form X CMP X +- Y to Y CMP 0.  */
-      if ((TREE_CODE (arg1) == PLUS_EXPR
-	   || TREE_CODE (arg1) == POINTER_PLUS_EXPR
-	   || TREE_CODE (arg1) == MINUS_EXPR)
-	  && operand_equal_p (tree_strip_nop_conversions (TREE_OPERAND (arg1,
-									0)),
-			      arg0, 0)
-	  && (INTEGRAL_TYPE_P (TREE_TYPE (arg1))
-	      || POINTER_TYPE_P (TREE_TYPE (arg1))))
-	{
-	  tree val = TREE_OPERAND (arg1, 1);
-	  val = fold_build2_loc (loc, code, type, val,
-				 build_int_cst (TREE_TYPE (val), 0));
-	  return omit_two_operands_loc (loc, type, val,
-					TREE_OPERAND (arg1, 0), arg0);
-	}
-
       /* If this is an EQ or NE comparison with zero and ARG0 is
 	 (1 << foo) & bar, convert it to (bar >> foo) & 1.  Both require
 	 two operations, but the latter can be done in one less insn
 	 on machines that have only two-operand insns or on which a
 	 constant cannot be the first operand.  */
       if (TREE_CODE (arg0) == BIT_AND_EXPR
 	  && integer_zerop (arg1))
 	{
 	  tree arg00 = TREE_OPERAND (arg0, 0);
 	  tree arg01 = TREE_OPERAND (arg0, 1);
@@ -10899,144 +10865,52 @@ fold_binary_loc (location_t loc,
     case GT_EXPR:
     case LE_EXPR:
     case GE_EXPR:
       tem = fold_comparison (loc, code, type, op0, op1);
       if (tem != NULL_TREE)
 	return tem;
 
       /* Transform comparisons of the form X +- C CMP X.  */
       if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
 	  && operand_equal_p (TREE_OPERAND (arg0, 0), arg1, 0)
-	  && ((TREE_CODE (TREE_OPERAND (arg0, 1)) == REAL_CST
-	       && !HONOR_SNANS (arg0))
-	      || (TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
-		  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))))
+	  && TREE_CODE (TREE_OPERAND (arg0, 1)) == REAL_CST
+	  && !HONOR_SNANS (arg0))
 	{
 	  tree arg01 = TREE_OPERAND (arg0, 1);
 	  enum tree_code code0 = TREE_CODE (arg0);
-	  int is_positive;
-
-	  if (TREE_CODE (arg01) == REAL_CST)
-	    is_positive = REAL_VALUE_NEGATIVE (TREE_REAL_CST (arg01)) ? -1 : 1;
-	  else
-	    is_positive = tree_int_cst_sgn (arg01);
+	  int is_positive = REAL_VALUE_NEGATIVE (TREE_REAL_CST (arg01)) ? -1 : 1;
 
 	  /* (X - c) > X becomes false.  */
 	  if (code == GT_EXPR
 	      && ((code0 == MINUS_EXPR && is_positive >= 0)
 		  || (code0 == PLUS_EXPR && is_positive <= 0)))
-	    {
-	      if (TREE_CODE (arg01) == INTEGER_CST
-		  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		fold_overflow_warning (("assuming signed overflow does not "
-					"occur when assuming that (X - c) > X "
-					"is always false"),
-				       WARN_STRICT_OVERFLOW_ALL);
-	      return constant_boolean_node (0, type);
-	    }
+	    return constant_boolean_node (0, type);
 
 	  /* Likewise (X + c) < X becomes false.  */
 	  if (code == LT_EXPR
 	      && ((code0 == PLUS_EXPR && is_positive >= 0)
 		  || (code0 == MINUS_EXPR && is_positive <= 0)))
-	    {
-	      if (TREE_CODE (arg01) == INTEGER_CST
-		  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		fold_overflow_warning (("assuming signed overflow does not "
-					"occur when assuming that "
-					"(X + c) < X is always false"),
-				       WARN_STRICT_OVERFLOW_ALL);
-	      return constant_boolean_node (0, type);
-	    }
+	    return constant_boolean_node (0, type);
 
 	  /* Convert (X - c) <= X to true.  */
 	  if (!HONOR_NANS (arg1)
 	      && code == LE_EXPR
 	      && ((code0 == MINUS_EXPR && is_positive >= 0)
 		  || (code0 == PLUS_EXPR && is_positive <= 0)))
-	    {
-	      if (TREE_CODE (arg01) == INTEGER_CST
-		  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		fold_overflow_warning (("assuming signed overflow does not "
-					"occur when assuming that "
-					"(X - c) <= X is always true"),
-				       WARN_STRICT_OVERFLOW_ALL);
-	      return constant_boolean_node (1, type);
-	    }
+	    return constant_boolean_node (1, type);
 
 	  /* Convert (X + c) >= X to true.  */
 	  if (!HONOR_NANS (arg1)
 	      && code == GE_EXPR
 	      && ((code0 == PLUS_EXPR && is_positive >= 0)
 		  || (code0 == MINUS_EXPR && is_positive <= 0)))
-	    {
-	      if (TREE_CODE (arg01) == INTEGER_CST
-		  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		fold_overflow_warning (("assuming signed overflow does not "
-					"occur when assuming that "
-					"(X + c) >= X is always true"),
-				       WARN_STRICT_OVERFLOW_ALL);
-	      return constant_boolean_node (1, type);
-	    }
-
-	  if (TREE_CODE (arg01) == INTEGER_CST)
-	    {
-	      /* Convert X + c > X and X - c < X to true for integers.  */
-	      if (code == GT_EXPR
-	          && ((code0 == PLUS_EXPR && is_positive > 0)
-		      || (code0 == MINUS_EXPR && is_positive < 0)))
-		{
-		  if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		    fold_overflow_warning (("assuming signed overflow does "
-					    "not occur when assuming that "
-					    "(X + c) > X is always true"),
-					   WARN_STRICT_OVERFLOW_ALL);
-		  return constant_boolean_node (1, type);
-		}
-
-	      if (code == LT_EXPR
-	          && ((code0 == MINUS_EXPR && is_positive > 0)
-		      || (code0 == PLUS_EXPR && is_positive < 0)))
-		{
-		  if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		    fold_overflow_warning (("assuming signed overflow does "
-					    "not occur when assuming that "
-					    "(X - c) < X is always true"),
-					   WARN_STRICT_OVERFLOW_ALL);
-		  return constant_boolean_node (1, type);
-		}
-
-	      /* Convert X + c <= X and X - c >= X to false for integers.  */
-	      if (code == LE_EXPR
-	          && ((code0 == PLUS_EXPR && is_positive > 0)
-		      || (code0 == MINUS_EXPR && is_positive < 0)))
-		{
-		  if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		    fold_overflow_warning (("assuming signed overflow does "
-					    "not occur when assuming that "
-					    "(X + c) <= X is always false"),
-					   WARN_STRICT_OVERFLOW_ALL);
-		  return constant_boolean_node (0, type);
-		}
-
-	      if (code == GE_EXPR
-	          && ((code0 == MINUS_EXPR && is_positive > 0)
-		      || (code0 == PLUS_EXPR && is_positive < 0)))
-		{
-		  if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)))
-		    fold_overflow_warning (("assuming signed overflow does "
-					    "not occur when assuming that "
-					    "(X - c) >= X is always false"),
-					   WARN_STRICT_OVERFLOW_ALL);
-		  return constant_boolean_node (0, type);
-		}
-	    }
+	    return constant_boolean_node (1, type);
 	}
 
       /* If we are comparing an ABS_EXPR with a constant, we can
 	 convert all the cases into explicit comparisons, but they may
 	 well not be faster than doing the ABS and one comparison.
 	 But ABS (X) <= C is a range comparison, which becomes a subtraction
 	 and a comparison, and is probably faster.  */
       if (code == LE_EXPR
 	  && TREE_CODE (arg1) == INTEGER_CST
 	  && TREE_CODE (arg0) == ABS_EXPR
Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 253558)
+++ gcc/match.pd	(working copy)
@@ -1268,20 +1268,58 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    (op @1 @0))))
 /* For equality and subtraction, this is also true with wrapping overflow.  */
 (for op (eq ne minus)
  (simplify
   (op (minus @2 @0) (minus @2 @1))
   (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
        && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
 	   || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))))
    (op @1 @0))))
 
+/* X + Y < Y is the same as X < 0 when there is no overflow.  */
+(for op (lt le gt ge)
+ (simplify
+  (op:c (plus:c@2 @0 @1) @1)
+  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+       && (CONSTANT_CLASS_P (@0) || single_use (@2)))
+   (op @0 { build_zero_cst (TREE_TYPE (@0)); }))))
+/* For equality, this is also true with wrapping overflow.  */
+(for op (eq ne)
+ (simplify
+  (op:c (nop_convert@3 (plus:c@2 @0 (convert1? @1))) (convert2? @1))
+  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+	   || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
+       && (CONSTANT_CLASS_P (@0) || (single_use (@2) && single_use (@3)))
+       && tree_nop_conversion_p (TREE_TYPE (@3), TREE_TYPE (@2))
+       && tree_nop_conversion_p (TREE_TYPE (@3), TREE_TYPE (@1)))
+   (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
+ (simplify
+  (op:c (nop_convert@3 (pointer_plus@2 (convert1? @0) @1)) (convert2? @0))
+  (if (tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0))
+       && tree_nop_conversion_p (TREE_TYPE (@3), TREE_TYPE (@0))
+       && (CONSTANT_CLASS_P (@1) || (single_use (@2) && single_use (@3))))
+   (op @1 { build_zero_cst (TREE_TYPE (@1)); }))))
+
+/* X - Y < X is the same as Y > 0 when there is no overflow.
+   For equality, this is also true with wrapping overflow.  */
+(for op (simple_comparison)
+ (simplify
+  (op:c @0 (minus@2 @0 @1))
+  (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+       && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+	   || ((op == EQ_EXPR || op == NE_EXPR)
+	       && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))))
+       && (CONSTANT_CLASS_P (@1) || single_use (@2)))
+   (op @1 { build_zero_cst (TREE_TYPE (@1)); }))))
+
 /* Transform:
  * (X / Y) == 0 -> X < Y if X, Y are unsigned.
  * (X / Y) != 0 -> X >= Y, if X, Y are unsigned.
  */
 (for cmp (eq ne)
      ocmp (lt ge)
  (simplify
   (cmp (trunc_div @0 @1) integer_zerop)
   (if (TYPE_UNSIGNED (TREE_TYPE (@0))
        && (VECTOR_TYPE_P (type) || !VECTOR_TYPE_P (TREE_TYPE (@0))))
@@ -3102,38 +3140,39 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    (with { tree tem = const_unop (NEGATE_EXPR, TREE_TYPE (@0), @1); }
     (if (tem && !TREE_OVERFLOW (tem))
      (scmp @0 { tem; }))))))
 
 /* Convert ABS_EXPR<x> == 0 or ABS_EXPR<x> != 0 to x == 0 or x != 0.  */
 (for op (eq ne)
  (simplify
   (op (abs @0) zerop@1)
   (op @0 @1)))
 
-/* From fold_sign_changed_comparison and fold_widened_comparison.  */
+/* From fold_sign_changed_comparison and fold_widened_comparison.
+   FIXME: the lack of symmetry is disturbing.  */
 (for cmp (simple_comparison)
  (simplify
   (cmp (convert@0 @00) (convert?@1 @10))
   (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
        /* Disable this optimization if we're casting a function pointer
 	  type on targets that require function pointer canonicalization.  */
        && !(targetm.have_canonicalize_funcptr_for_compare ()
 	    && TREE_CODE (TREE_TYPE (@00)) == POINTER_TYPE
 	    && TREE_CODE (TREE_TYPE (TREE_TYPE (@00))) == FUNCTION_TYPE)
        && single_use (@0))
    (if (TYPE_PRECISION (TREE_TYPE (@00)) == TYPE_PRECISION (TREE_TYPE (@0))
 	&& (TREE_CODE (@10) == INTEGER_CST
-	    || (@1 != @10 && types_match (TREE_TYPE (@10), TREE_TYPE (@00))))
+	    || @1 != @10)
 	&& (TYPE_UNSIGNED (TREE_TYPE (@00)) == TYPE_UNSIGNED (TREE_TYPE (@0))
 	    || cmp == NE_EXPR
 	    || cmp == EQ_EXPR)
-	&& (POINTER_TYPE_P (TREE_TYPE (@00)) == POINTER_TYPE_P (TREE_TYPE (@0))))
+	&& !POINTER_TYPE_P (TREE_TYPE (@00)))
     /* ???  The special-casing of INTEGER_CST conversion was in the original
        code and here to avoid a spurious overflow flag on the resulting
        constant which fold_convert produces.  */
     (if (TREE_CODE (@1) == INTEGER_CST)
      (cmp @00 { force_fit_type (TREE_TYPE (@00), wi::to_widest (@1), 0,
 				TREE_OVERFLOW (@1)); })
      (cmp @00 (convert @1)))
 
     (if (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (TREE_TYPE (@00)))
      /* If possible, express the comparison in the shorter mode.  */
Index: gcc/testsuite/gcc.dg/Wstrict-overflow-7.c
===================================================================
--- gcc/testsuite/gcc.dg/Wstrict-overflow-7.c	(revision 253558)
+++ gcc/testsuite/gcc.dg/Wstrict-overflow-7.c	(working copy)
@@ -1,10 +1,10 @@
 /* { dg-do compile } */
 /* { dg-options "-fstrict-overflow -O2 -Wstrict-overflow" } */
 
 /* Source: Ian Lance Taylor.  */
 
 int
 foo (int i)
 {
-  return i + 10 > i; /* { dg-warning "assuming signed overflow does not occur" "correct warning" } */
+  return i + 10 > i; /* { dg-warning "assuming signed overflow does not occur" "correct warning" { xfail *-*-* } } */
 }
Index: gcc/testsuite/gcc.dg/pragma-diag-3.c
===================================================================
--- gcc/testsuite/gcc.dg/pragma-diag-3.c	(revision 253558)
+++ gcc/testsuite/gcc.dg/pragma-diag-3.c	(working copy)
@@ -8,21 +8,21 @@
 void testing2() {
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wstrict-overflow"
   int j = 4;
   j + 4 < j;
 #pragma GCC diagnostic pop
 }
 
 void testing3() {
   int k = 4;
-  k + 4 < k; /* { dg-error "overflow" } */
+  k + 4 < k; /* { dg-error "overflow" "" { xfail *-*-* } } */
 }
 
 int bar()
 {
   unsigned x = 0;
   int y = 1;
 
   /* generates an error - ok */
   x += x < y ? 1 : 0; /* { dg-error "comparison" } */
 

Reply via email to