public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
@ 2016-04-24 17:14 Marc Glisse
  2016-04-26 11:58 ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Glisse @ 2016-04-24 17:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2108 bytes --]

Hello,

trying to move a first pattern from fold_comparison.

I first tried without single_use. It brought the number of 'free' in 
g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2 SMS 
(I don't think the generated code was worse, maybe even better, but I 
don't know ppc asm), broke Wstrict-overflow-18.c (the optimization moved 
from VRP to CCP if I remember correctly), and caused IVOPTS to make a mess 
in guality/pr54693-2.c (much longer code, and many <optimized> debug 
variables). If someone wants to drop the single_use, they can work on that 
after this patch is in.

The conditions do not exactly match the ones in fold-const.c, but I guess 
they are close. The warning in the constant case was missing in 
fold_comparison, but present in VRP, so I had to add it not to regress.

I don't think we were warning much from match.pd. I can't say that I am a 
big fan of those strict overflow warnings, but I expect people would 
complain if we just dropped the existing ones when moving the transforms 
to match.pd?

I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS || 
TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict 
overflow), so I went with some kind of complement we use elsewhere. Now 
that I am writing this, I don't remember why I didn't just add 
-fstrict-overflow to the testcase, or xfail it at -O1. The saturating case 
could be handled as long as the constant is not an extremum, but I don't 
think we really handle saturating integers anyway.

I split the equality case, because it was already getting ugly.

Bootstrap+regtest on powerpc64le-unknown-linux-gnu.

2016-04-25  Marc Glisse  <marc.glisse@inria.fr>

gcc/
 	* fold-const.h: Include flag-types.h.
 	(fold_overflow_warning): Declare.
 	* fold-const.c (fold_overflow_warning): Make non-static.
 	(fold_comparison): Move the transformation of X +- C1 CMP C2
 	into X CMP C2 -+ C1 ...
 	* match.pd: ... here.
 	* tree-ssa-forwprop.c (execute): Protect fold_stmt with
 	fold_defer_overflow_warnings.

gcc/testsuite/
 	* gcc.dg/tree-ssa/20040305-1.c: Adjust.


-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10983 bytes --]

Index: trunk4/gcc/fold-const.c
===================================================================
--- trunk4/gcc/fold-const.c	(revision 235384)
+++ trunk4/gcc/fold-const.c	(working copy)
@@ -290,21 +290,21 @@ fold_undefer_and_ignore_overflow_warning
 
 bool
 fold_deferring_overflow_warnings_p (void)
 {
   return fold_deferring_overflow_warnings > 0;
 }
 
 /* This is called when we fold something based on the fact that signed
    overflow is undefined.  */
 
-static void
+void
 fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code wc)
 {
   if (fold_deferring_overflow_warnings > 0)
     {
       if (fold_deferred_overflow_warning == NULL
 	  || wc < fold_deferred_overflow_code)
 	{
 	  fold_deferred_overflow_warning = gmsgid;
 	  fold_deferred_overflow_code = wc;
 	}
@@ -8366,89 +8366,20 @@ fold_comparison (location_t loc, enum tr
 {
   const bool equality_code = (code == EQ_EXPR || code == NE_EXPR);
   tree arg0, arg1, tem;
 
   arg0 = op0;
   arg1 = op1;
 
   STRIP_SIGN_NOPS (arg0);
   STRIP_SIGN_NOPS (arg1);
 
-  /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
-  if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
-      && (equality_code
-	  || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg0))
-	      && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))))
-      && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
-      && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1))
-      && TREE_CODE (arg1) == INTEGER_CST
-      && !TREE_OVERFLOW (arg1))
-    {
-      const enum tree_code
-	reverse_op = TREE_CODE (arg0) == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR;
-      tree const1 = TREE_OPERAND (arg0, 1);
-      tree const2 = fold_convert_loc (loc, TREE_TYPE (const1), arg1);
-      tree variable = TREE_OPERAND (arg0, 0);
-      tree new_const = int_const_binop (reverse_op, const2, const1);
-
-      /* If the constant operation overflowed this can be
-	 simplified as a comparison against INT_MAX/INT_MIN.  */
-      if (TREE_OVERFLOW (new_const)
-	  && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
-	{
-	  int const1_sgn = tree_int_cst_sgn (const1);
-	  enum tree_code code2 = code;
-
-	  /* Get the sign of the constant on the lhs if the
-	     operation were VARIABLE + CONST1.  */
-	  if (TREE_CODE (arg0) == MINUS_EXPR)
-	    const1_sgn = -const1_sgn;
-
-	  /* The sign of the constant determines if we overflowed
-	     INT_MAX (const1_sgn == -1) or INT_MIN (const1_sgn == 1).
-	     Canonicalize to the INT_MIN overflow by swapping the comparison
-	     if necessary.  */
-	  if (const1_sgn == -1)
-	    code2 = swap_tree_comparison (code);
-
-	  /* We now can look at the canonicalized case
-	       VARIABLE + 1  CODE2  INT_MIN
-	     and decide on the result.  */
-	  switch (code2)
-	    {
-	    case EQ_EXPR:
-	    case LT_EXPR:
-	    case LE_EXPR:
-	      return
-		omit_one_operand_loc (loc, type, boolean_false_node, variable);
-
-	    case NE_EXPR:
-	    case GE_EXPR:
-	    case GT_EXPR:
-	      return
-		omit_one_operand_loc (loc, type, boolean_true_node, variable);
-
-	    default:
-	      gcc_unreachable ();
-	    }
-	}
-      else
-	{
-	  if (!equality_code)
-	    fold_overflow_warning ("assuming signed overflow does not occur "
-				   "when changing X +- C1 cmp C2 to "
-				   "X cmp C2 -+ C1",
-				   WARN_STRICT_OVERFLOW_COMPARISON);
-	  return fold_build2_loc (loc, code, type, variable, new_const);
-	}
-    }
-
   /* For comparisons of pointers we can decompose it to a compile time
      comparison of the base objects and the offsets into the object.
      This requires at least one operand being an ADDR_EXPR or a
      POINTER_PLUS_EXPR to do more than the operand_equal_p test below.  */
   if (POINTER_TYPE_P (TREE_TYPE (arg0))
       && (TREE_CODE (arg0) == ADDR_EXPR
 	  || TREE_CODE (arg1) == ADDR_EXPR
 	  || TREE_CODE (arg0) == POINTER_PLUS_EXPR
 	  || TREE_CODE (arg1) == POINTER_PLUS_EXPR))
     {
Index: trunk4/gcc/fold-const.h
===================================================================
--- trunk4/gcc/fold-const.h	(revision 235384)
+++ trunk4/gcc/fold-const.h	(working copy)
@@ -13,20 +13,22 @@ WARRANTY; without even the implied warra
 FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 for more details.
 
 You should have received a copy of the GNU General Public License
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #ifndef GCC_FOLD_CONST_H
 #define GCC_FOLD_CONST_H
 
+#include <flag-types.h>
+
 /* Non-zero if we are folding constants inside an initializer; zero
    otherwise.  */
 extern int folding_initializer;
 
 /* Convert between trees and native memory representation.  */
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
 extern tree native_interpret_expr (tree, const unsigned char *, int);
 
 /* Fold constants as much as possible in an expression.
    Returns the simplified expression.
@@ -79,20 +81,21 @@ extern bool fold_convertible_p (const_tr
    fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
 extern tree fold_convert_loc (location_t, tree, tree);
 extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree, tree);
 extern tree fold_ignored_result (tree);
 extern tree fold_abs_const (tree, tree);
 extern tree fold_indirect_ref_1 (location_t, tree, tree);
 extern void fold_defer_overflow_warnings (void);
 extern void fold_undefer_overflow_warnings (bool, const gimple *, int);
 extern void fold_undefer_and_ignore_overflow_warnings (void);
 extern bool fold_deferring_overflow_warnings_p (void);
+extern void fold_overflow_warning (const char*, enum warn_strict_overflow_code);
 extern int operand_equal_p (const_tree, const_tree, unsigned int);
 extern int multiple_of_p (tree, const_tree, const_tree);
 #define omit_one_operand(T1,T2,T3)\
    omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
 extern tree omit_one_operand_loc (location_t, tree, tree, tree);
 #define omit_two_operands(T1,T2,T3,T4)\
    omit_two_operands_loc (UNKNOWN_LOCATION, T1, T2, T3, T4)
 extern tree omit_two_operands_loc (location_t, tree, tree, tree, tree);
 #define invert_truthvalue(T)\
    invert_truthvalue_loc (UNKNOWN_LOCATION, T)
Index: trunk4/gcc/match.pd
===================================================================
--- trunk4/gcc/match.pd	(revision 235384)
+++ trunk4/gcc/match.pd	(working copy)
@@ -3071,10 +3071,54 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
  /* signbit(x) -> 0 if x is nonnegative.  */
  (SIGNBIT tree_expr_nonnegative_p@0)
  { integer_zero_node; })
 
 (simplify
  /* signbit(x) -> x<0 if x doesn't have signed zeros.  */
  (SIGNBIT @0)
  (if (!HONOR_SIGNED_ZEROS (@0))
   (convert (lt @0 { build_real (TREE_TYPE (@0), dconst0); }))))
+
+/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
+(for cmp (eq ne)
+ (for op (plus minus)
+      rop (minus plus)
+  (simplify
+   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
+   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
+	&& !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
+	&& !TYPE_OVERFLOW_TRAPS (TREE_TYPE (@0))
+	&& !TYPE_SATURATING (TREE_TYPE (@0)))
+    (with { tree res = int_const_binop (rop, @2, @1); }
+     (if (TREE_OVERFLOW (res))
+      { constant_boolean_node (cmp == NE_EXPR, type); }
+      (if (single_use (@3))
+       (cmp @0 { res; }))))))))
+(for cmp (lt le gt ge)
+ (for op (plus minus)
+      rop (minus plus)
+  (simplify
+   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
+   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
+	&& TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
+    (with { tree res = int_const_binop (rop, @2, @1); }
+     (if (TREE_OVERFLOW (res))
+      {
+	fold_overflow_warning (("assuming signed overflow does not occur "
+				"when simplifying conditional to constant"),
+			       WARN_STRICT_OVERFLOW_CONDITIONAL);
+        bool less = cmp == LE_EXPR || cmp == LT_EXPR;
+	/* wi::ges_p (@2, 0) should be sufficient for a signed type.  */
+	bool ovf_high = wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1)))
+			!= (op == MINUS_EXPR);
+	constant_boolean_node (less == ovf_high, type);
+      }
+      (if (single_use (@3))
+       (with
+	{
+	  fold_overflow_warning (("assuming signed overflow does not occur "
+				  "when changing X +- C1 cmp C2 to "
+				  "X cmp C2 -+ C1"),
+				 WARN_STRICT_OVERFLOW_COMPARISON);
+	}
+	(cmp @0 { res; })))))))))
Index: trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c
===================================================================
--- trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c	(revision 235384)
+++ trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c	(working copy)
@@ -16,15 +16,15 @@ void foo(int edx, int eax)
     }
   if (eax == 100)
     {
       if (-- edx == 0)
         afred[0] = 2;
     }
 }
  
 
 /* Verify that we did a forward propagation.  */
-/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
+/* { dg-final { scan-tree-dump-times "gimple_simplified" 1 "forwprop1"} } */
 
 /* After cddce we should have two IF statements remaining as the other
    two tests can be threaded.  */
 /* { dg-final { scan-tree-dump-times "if " 2 "cddce2"} } */
Index: trunk4/gcc/tree-ssa-forwprop.c
===================================================================
--- trunk4/gcc/tree-ssa-forwprop.c	(revision 235384)
+++ trunk4/gcc/tree-ssa-forwprop.c	(working copy)
@@ -2299,37 +2299,41 @@ pass_forwprop::execute (function *fun)
 	{
 	  gimple *stmt = gsi_stmt (gsi);
 	  gimple *orig_stmt = stmt;
 	  bool changed = false;
 	  bool was_noreturn = (is_gimple_call (stmt)
 			       && gimple_call_noreturn_p (stmt));
 
 	  /* Mark stmt as potentially needing revisiting.  */
 	  gimple_set_plf (stmt, GF_PLF_1, false);
 
+	  fold_defer_overflow_warnings ();
 	  if (fold_stmt (&gsi, fwprop_ssa_val))
 	    {
 	      changed = true;
 	      stmt = gsi_stmt (gsi);
 	      if (maybe_clean_or_replace_eh_stmt (orig_stmt, stmt))
 		bitmap_set_bit (to_purge, bb->index);
 	      if (!was_noreturn
 		  && is_gimple_call (stmt) && gimple_call_noreturn_p (stmt))
 		to_fixup.safe_push (stmt);
 	      /* Cleanup the CFG if we simplified a condition to
 	         true or false.  */
 	      if (gcond *cond = dyn_cast <gcond *> (stmt))
 		if (gimple_cond_true_p (cond)
 		    || gimple_cond_false_p (cond))
 		  cfg_changed = true;
 	      update_stmt (stmt);
+	      fold_undefer_overflow_warnings (!gimple_no_warning_p (stmt), stmt, 0);
 	    }
+	  else
+	    fold_undefer_overflow_warnings (false, NULL, 0);
 
 	  switch (gimple_code (stmt))
 	    {
 	    case GIMPLE_ASSIGN:
 	      {
 		tree rhs1 = gimple_assign_rhs1 (stmt);
 		enum tree_code code = gimple_assign_rhs_code (stmt);
 
 		if (code == COND_EXPR
 		    || code == VEC_COND_EXPR)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-24 17:14 Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd Marc Glisse
@ 2016-04-26 11:58 ` Richard Biener
  2016-04-26 20:28   ` Marc Glisse
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2016-04-26 11:58 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Sun, Apr 24, 2016 at 7:14 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> trying to move a first pattern from fold_comparison.
>
> I first tried without single_use. It brought the number of 'free' in
> g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2 SMS
> (I don't think the generated code was worse, maybe even better, but I don't
> know ppc asm), broke Wstrict-overflow-18.c (the optimization moved from VRP
> to CCP if I remember correctly), and caused IVOPTS to make a mess in
> guality/pr54693-2.c (much longer code, and many <optimized> debug
> variables). If someone wants to drop the single_use, they can work on that
> after this patch is in.
>
> The conditions do not exactly match the ones in fold-const.c, but I guess
> they are close. The warning in the constant case was missing in
> fold_comparison, but present in VRP, so I had to add it not to regress.
>
> I don't think we were warning much from match.pd. I can't say that I am a
> big fan of those strict overflow warnings, but I expect people would
> complain if we just dropped the existing ones when moving the transforms to
> match.pd?

I just dropped them for patterns I moved (luckily we didn't have
testcases sofar ;))

If we really want to warn from match.pd then you should do the defer/undefer
stuff in fold_stmt itself (same condition I guess) and defer/undefer without
warning in gimple_fold_stmt_to_constant_1.

So you actually ran into a testcase that expected the warning?

> I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS ||
> TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict
> overflow), so I went with some kind of complement we use elsewhere.

I think I prefer to move things 1:1 (unless sth regresses) and fix bugs in the
fold-const.c variant as followup (possibly also adding testcases).

IMHO -fno-strict-overflow needs to go.  It has very wary designed semantics
(ops neither wrap nor have undefined overflow).

> Now that
> I am writing this, I don't remember why I didn't just add -fstrict-overflow
> to the testcase, or xfail it at -O1. The saturating case could be handled as
> long as the constant is not an extremum, but I don't think we really handle
> saturating integers anyway.
>
> I split the equality case, because it was already getting ugly.

That's fine.

Thanks,
Richard.

> Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
>
> 2016-04-25  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/
>         * fold-const.h: Include flag-types.h.
>         (fold_overflow_warning): Declare.
>         * fold-const.c (fold_overflow_warning): Make non-static.
>         (fold_comparison): Move the transformation of X +- C1 CMP C2
>         into X CMP C2 -+ C1 ...
>         * match.pd: ... here.
>         * tree-ssa-forwprop.c (execute): Protect fold_stmt with
>         fold_defer_overflow_warnings.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/20040305-1.c: Adjust.
>
>
> --
> Marc Glisse
> Index: trunk4/gcc/fold-const.c
> ===================================================================
> --- trunk4/gcc/fold-const.c     (revision 235384)
> +++ trunk4/gcc/fold-const.c     (working copy)
> @@ -290,21 +290,21 @@ fold_undefer_and_ignore_overflow_warning
>
>  bool
>  fold_deferring_overflow_warnings_p (void)
>  {
>    return fold_deferring_overflow_warnings > 0;
>  }
>
>  /* This is called when we fold something based on the fact that signed
>     overflow is undefined.  */
>
> -static void
> +void
>  fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code
> wc)
>  {
>    if (fold_deferring_overflow_warnings > 0)
>      {
>        if (fold_deferred_overflow_warning == NULL
>           || wc < fold_deferred_overflow_code)
>         {
>           fold_deferred_overflow_warning = gmsgid;
>           fold_deferred_overflow_code = wc;
>         }
> @@ -8366,89 +8366,20 @@ fold_comparison (location_t loc, enum tr
>  {
>    const bool equality_code = (code == EQ_EXPR || code == NE_EXPR);
>    tree arg0, arg1, tem;
>
>    arg0 = op0;
>    arg1 = op1;
>
>    STRIP_SIGN_NOPS (arg0);
>    STRIP_SIGN_NOPS (arg1);
>
> -  /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.
> */
> -  if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
> -      && (equality_code
> -         || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg0))
> -             && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))))
> -      && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
> -      && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1))
> -      && TREE_CODE (arg1) == INTEGER_CST
> -      && !TREE_OVERFLOW (arg1))
> -    {
> -      const enum tree_code
> -       reverse_op = TREE_CODE (arg0) == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR;
> -      tree const1 = TREE_OPERAND (arg0, 1);
> -      tree const2 = fold_convert_loc (loc, TREE_TYPE (const1), arg1);
> -      tree variable = TREE_OPERAND (arg0, 0);
> -      tree new_const = int_const_binop (reverse_op, const2, const1);
> -
> -      /* If the constant operation overflowed this can be
> -        simplified as a comparison against INT_MAX/INT_MIN.  */
> -      if (TREE_OVERFLOW (new_const)
> -         && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
> -       {
> -         int const1_sgn = tree_int_cst_sgn (const1);
> -         enum tree_code code2 = code;
> -
> -         /* Get the sign of the constant on the lhs if the
> -            operation were VARIABLE + CONST1.  */
> -         if (TREE_CODE (arg0) == MINUS_EXPR)
> -           const1_sgn = -const1_sgn;
> -
> -         /* The sign of the constant determines if we overflowed
> -            INT_MAX (const1_sgn == -1) or INT_MIN (const1_sgn == 1).
> -            Canonicalize to the INT_MIN overflow by swapping the comparison
> -            if necessary.  */
> -         if (const1_sgn == -1)
> -           code2 = swap_tree_comparison (code);
> -
> -         /* We now can look at the canonicalized case
> -              VARIABLE + 1  CODE2  INT_MIN
> -            and decide on the result.  */
> -         switch (code2)
> -           {
> -           case EQ_EXPR:
> -           case LT_EXPR:
> -           case LE_EXPR:
> -             return
> -               omit_one_operand_loc (loc, type, boolean_false_node,
> variable);
> -
> -           case NE_EXPR:
> -           case GE_EXPR:
> -           case GT_EXPR:
> -             return
> -               omit_one_operand_loc (loc, type, boolean_true_node,
> variable);
> -
> -           default:
> -             gcc_unreachable ();
> -           }
> -       }
> -      else
> -       {
> -         if (!equality_code)
> -           fold_overflow_warning ("assuming signed overflow does not occur
> "
> -                                  "when changing X +- C1 cmp C2 to "
> -                                  "X cmp C2 -+ C1",
> -                                  WARN_STRICT_OVERFLOW_COMPARISON);
> -         return fold_build2_loc (loc, code, type, variable, new_const);
> -       }
> -    }
> -
>    /* For comparisons of pointers we can decompose it to a compile time
>       comparison of the base objects and the offsets into the object.
>       This requires at least one operand being an ADDR_EXPR or a
>       POINTER_PLUS_EXPR to do more than the operand_equal_p test below.  */
>    if (POINTER_TYPE_P (TREE_TYPE (arg0))
>        && (TREE_CODE (arg0) == ADDR_EXPR
>           || TREE_CODE (arg1) == ADDR_EXPR
>           || TREE_CODE (arg0) == POINTER_PLUS_EXPR
>           || TREE_CODE (arg1) == POINTER_PLUS_EXPR))
>      {
> Index: trunk4/gcc/fold-const.h
> ===================================================================
> --- trunk4/gcc/fold-const.h     (revision 235384)
> +++ trunk4/gcc/fold-const.h     (working copy)
> @@ -13,20 +13,22 @@ WARRANTY; without even the implied warra
>  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>  for more details.
>
>  You should have received a copy of the GNU General Public License
>  along with GCC; see the file COPYING3.  If not see
>  <http://www.gnu.org/licenses/>.  */
>
>  #ifndef GCC_FOLD_CONST_H
>  #define GCC_FOLD_CONST_H
>
> +#include <flag-types.h>
> +
>  /* Non-zero if we are folding constants inside an initializer; zero
>     otherwise.  */
>  extern int folding_initializer;
>
>  /* Convert between trees and native memory representation.  */
>  extern int native_encode_expr (const_tree, unsigned char *, int, int off =
> -1);
>  extern tree native_interpret_expr (tree, const unsigned char *, int);
>
>  /* Fold constants as much as possible in an expression.
>     Returns the simplified expression.
> @@ -79,20 +81,21 @@ extern bool fold_convertible_p (const_tr
>     fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
>  extern tree fold_convert_loc (location_t, tree, tree);
>  extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree,
> tree);
>  extern tree fold_ignored_result (tree);
>  extern tree fold_abs_const (tree, tree);
>  extern tree fold_indirect_ref_1 (location_t, tree, tree);
>  extern void fold_defer_overflow_warnings (void);
>  extern void fold_undefer_overflow_warnings (bool, const gimple *, int);
>  extern void fold_undefer_and_ignore_overflow_warnings (void);
>  extern bool fold_deferring_overflow_warnings_p (void);
> +extern void fold_overflow_warning (const char*, enum
> warn_strict_overflow_code);
>  extern int operand_equal_p (const_tree, const_tree, unsigned int);
>  extern int multiple_of_p (tree, const_tree, const_tree);
>  #define omit_one_operand(T1,T2,T3)\
>     omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
>  extern tree omit_one_operand_loc (location_t, tree, tree, tree);
>  #define omit_two_operands(T1,T2,T3,T4)\
>     omit_two_operands_loc (UNKNOWN_LOCATION, T1, T2, T3, T4)
>  extern tree omit_two_operands_loc (location_t, tree, tree, tree, tree);
>  #define invert_truthvalue(T)\
>     invert_truthvalue_loc (UNKNOWN_LOCATION, T)
> Index: trunk4/gcc/match.pd
> ===================================================================
> --- trunk4/gcc/match.pd (revision 235384)
> +++ trunk4/gcc/match.pd (working copy)
> @@ -3071,10 +3071,54 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (simplify
>   /* signbit(x) -> 0 if x is nonnegative.  */
>   (SIGNBIT tree_expr_nonnegative_p@0)
>   { integer_zero_node; })
>
>  (simplify
>   /* signbit(x) -> x<0 if x doesn't have signed zeros.  */
>   (SIGNBIT @0)
>   (if (!HONOR_SIGNED_ZEROS (@0))
>    (convert (lt @0 { build_real (TREE_TYPE (@0), dconst0); }))))
> +
> +/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
> +(for cmp (eq ne)
> + (for op (plus minus)
> +      rop (minus plus)
> +  (simplify
> +   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
> +   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
> +       && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
> +       && !TYPE_OVERFLOW_TRAPS (TREE_TYPE (@0))
> +       && !TYPE_SATURATING (TREE_TYPE (@0)))
> +    (with { tree res = int_const_binop (rop, @2, @1); }
> +     (if (TREE_OVERFLOW (res))
> +      { constant_boolean_node (cmp == NE_EXPR, type); }
> +      (if (single_use (@3))
> +       (cmp @0 { res; }))))))))
> +(for cmp (lt le gt ge)
> + (for op (plus minus)
> +      rop (minus plus)
> +  (simplify
> +   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
> +   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
> +       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
> +    (with { tree res = int_const_binop (rop, @2, @1); }
> +     (if (TREE_OVERFLOW (res))
> +      {
> +       fold_overflow_warning (("assuming signed overflow does not occur "
> +                               "when simplifying conditional to constant"),
> +                              WARN_STRICT_OVERFLOW_CONDITIONAL);
> +        bool less = cmp == LE_EXPR || cmp == LT_EXPR;
> +       /* wi::ges_p (@2, 0) should be sufficient for a signed type.  */
> +       bool ovf_high = wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1)))
> +                       != (op == MINUS_EXPR);
> +       constant_boolean_node (less == ovf_high, type);
> +      }
> +      (if (single_use (@3))
> +       (with
> +       {
> +         fold_overflow_warning (("assuming signed overflow does not occur "
> +                                 "when changing X +- C1 cmp C2 to "
> +                                 "X cmp C2 -+ C1"),
> +                                WARN_STRICT_OVERFLOW_COMPARISON);
> +       }
> +       (cmp @0 { res; })))))))))
> Index: trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c
> ===================================================================
> --- trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c   (revision 235384)
> +++ trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c   (working copy)
> @@ -16,15 +16,15 @@ void foo(int edx, int eax)
>      }
>    if (eax == 100)
>      {
>        if (-- edx == 0)
>          afred[0] = 2;
>      }
>  }
>
>
>  /* Verify that we did a forward propagation.  */
> -/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
> +/* { dg-final { scan-tree-dump-times "gimple_simplified" 1 "forwprop1"} }
> */
>
>  /* After cddce we should have two IF statements remaining as the other
>     two tests can be threaded.  */
>  /* { dg-final { scan-tree-dump-times "if " 2 "cddce2"} } */
> Index: trunk4/gcc/tree-ssa-forwprop.c
> ===================================================================
> --- trunk4/gcc/tree-ssa-forwprop.c      (revision 235384)
> +++ trunk4/gcc/tree-ssa-forwprop.c      (working copy)
> @@ -2299,37 +2299,41 @@ pass_forwprop::execute (function *fun)
>         {
>           gimple *stmt = gsi_stmt (gsi);
>           gimple *orig_stmt = stmt;
>           bool changed = false;
>           bool was_noreturn = (is_gimple_call (stmt)
>                                && gimple_call_noreturn_p (stmt));
>
>           /* Mark stmt as potentially needing revisiting.  */
>           gimple_set_plf (stmt, GF_PLF_1, false);
>
> +         fold_defer_overflow_warnings ();
>           if (fold_stmt (&gsi, fwprop_ssa_val))
>             {
>               changed = true;
>               stmt = gsi_stmt (gsi);
>               if (maybe_clean_or_replace_eh_stmt (orig_stmt, stmt))
>                 bitmap_set_bit (to_purge, bb->index);
>               if (!was_noreturn
>                   && is_gimple_call (stmt) && gimple_call_noreturn_p (stmt))
>                 to_fixup.safe_push (stmt);
>               /* Cleanup the CFG if we simplified a condition to
>                  true or false.  */
>               if (gcond *cond = dyn_cast <gcond *> (stmt))
>                 if (gimple_cond_true_p (cond)
>                     || gimple_cond_false_p (cond))
>                   cfg_changed = true;
>               update_stmt (stmt);
> +             fold_undefer_overflow_warnings (!gimple_no_warning_p (stmt),
> stmt, 0);
>             }
> +         else
> +           fold_undefer_overflow_warnings (false, NULL, 0);
>
>           switch (gimple_code (stmt))
>             {
>             case GIMPLE_ASSIGN:
>               {
>                 tree rhs1 = gimple_assign_rhs1 (stmt);
>                 enum tree_code code = gimple_assign_rhs_code (stmt);
>
>                 if (code == COND_EXPR
>                     || code == VEC_COND_EXPR)
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-26 11:58 ` Richard Biener
@ 2016-04-26 20:28   ` Marc Glisse
  2016-04-27  5:34     ` Marc Glisse
  2016-04-27 11:52     ` Richard Biener
  0 siblings, 2 replies; 10+ messages in thread
From: Marc Glisse @ 2016-04-26 20:28 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Tue, 26 Apr 2016, Richard Biener wrote:

> On Sun, Apr 24, 2016 at 7:14 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> trying to move a first pattern from fold_comparison.
>>
>> I first tried without single_use. It brought the number of 'free' in
>> g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2 SMS
>> (I don't think the generated code was worse, maybe even better, but I don't
>> know ppc asm), broke Wstrict-overflow-18.c (the optimization moved from VRP
>> to CCP if I remember correctly), and caused IVOPTS to make a mess in
>> guality/pr54693-2.c (much longer code, and many <optimized> debug
>> variables). If someone wants to drop the single_use, they can work on that
>> after this patch is in.
>>
>> The conditions do not exactly match the ones in fold-const.c, but I guess
>> they are close. The warning in the constant case was missing in
>> fold_comparison, but present in VRP, so I had to add it not to regress.
>>
>> I don't think we were warning much from match.pd. I can't say that I am a
>> big fan of those strict overflow warnings, but I expect people would
>> complain if we just dropped the existing ones when moving the transforms to
>> match.pd?
>
> I just dropped them for patterns I moved (luckily we didn't have
> testcases sofar ;))
>
> If we really want to warn from match.pd then you should do the defer/undefer
> stuff in fold_stmt itself (same condition I guess) and defer/undefer without
> warning in gimple_fold_stmt_to_constant_1.

Moving it to fold_stmt_1 seems like a good idea, much better than putting 
it in forwprop. Looking at gimple_fold_stmt_to_constant_1 on the other 
hand, I have some concerns. If we do not warn for 
gimple_fold_stmt_to_constant_1, we are going to miss some warnings (I 
believe there is at least one testcase that will break, where VRP 
currently warns but CCP will come first). On the other hand, 
gimple_fold_stmt_to_constant_1 doesn't do much validation on its return 
value, each caller has their own idea of which results are acceptable, so 
it is only in the (many) callers that we can defer/undefer, or we might 
warn for transformations that we don't actually perform. CCP already has 
the defer/undefer calls around ccp_fold and thus 
gimple_fold_stmt_to_constant_1.

> So you actually ran into a testcase that expected the warning?

Several of them if I remember correctly...

>> I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS ||
>> TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict
>> overflow), so I went with some kind of complement we use elsewhere.
>
> I think I prefer to move things 1:1 (unless sth regresses) and fix bugs in the
> fold-const.c variant as followup (possibly also adding testcases).

Well, yes, but things do indeed regress quite regularly when moving things 
1:1 :-( At least unless we add a big (if (GENERIC) ...) around the 
transformation.

> IMHO -fno-strict-overflow needs to go.  It has very wary designed semantics
> (ops neither wrap nor have undefined overflow).

Maybe make it an alias for -fwrapv?

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-26 20:28   ` Marc Glisse
@ 2016-04-27  5:34     ` Marc Glisse
  2016-04-27 12:02       ` Richard Biener
  2016-04-27 11:52     ` Richard Biener
  1 sibling, 1 reply; 10+ messages in thread
From: Marc Glisse @ 2016-04-27  5:34 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3542 bytes --]

Here is the current patch (passed regtest), after moving defer/undefer 
from forwprop to fold_stmt_1. I am not sure if checking no_warning at the 
end of fold_stmt_1 is safe, or if I should save its value at the beginning 
of the function, in case some of the transformations clear it.

On Tue, 26 Apr 2016, Marc Glisse wrote:

> On Tue, 26 Apr 2016, Richard Biener wrote:
>
>> On Sun, Apr 24, 2016 at 7:14 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>> Hello,
>>> 
>>> trying to move a first pattern from fold_comparison.
>>> 
>>> I first tried without single_use. It brought the number of 'free' in
>>> g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2 SMS
>>> (I don't think the generated code was worse, maybe even better, but I 
>>> don't
>>> know ppc asm), broke Wstrict-overflow-18.c (the optimization moved from 
>>> VRP
>>> to CCP if I remember correctly), and caused IVOPTS to make a mess in
>>> guality/pr54693-2.c (much longer code, and many <optimized> debug
>>> variables). If someone wants to drop the single_use, they can work on that
>>> after this patch is in.
>>> 
>>> The conditions do not exactly match the ones in fold-const.c, but I guess
>>> they are close. The warning in the constant case was missing in
>>> fold_comparison, but present in VRP, so I had to add it not to regress.
>>> 
>>> I don't think we were warning much from match.pd. I can't say that I am a
>>> big fan of those strict overflow warnings, but I expect people would
>>> complain if we just dropped the existing ones when moving the transforms 
>>> to
>>> match.pd?
>> 
>> I just dropped them for patterns I moved (luckily we didn't have
>> testcases sofar ;))
>> 
>> If we really want to warn from match.pd then you should do the 
>> defer/undefer
>> stuff in fold_stmt itself (same condition I guess) and defer/undefer 
>> without
>> warning in gimple_fold_stmt_to_constant_1.
>
> Moving it to fold_stmt_1 seems like a good idea, much better than putting it 
> in forwprop. Looking at gimple_fold_stmt_to_constant_1 on the other hand, I 
> have some concerns. If we do not warn for gimple_fold_stmt_to_constant_1, we 
> are going to miss some warnings (I believe there is at least one testcase 
> that will break, where VRP currently warns but CCP will come first). On the 
> other hand, gimple_fold_stmt_to_constant_1 doesn't do much validation on its 
> return value, each caller has their own idea of which results are acceptable, 
> so it is only in the (many) callers that we can defer/undefer, or we might 
> warn for transformations that we don't actually perform. CCP already has the 
> defer/undefer calls around ccp_fold and thus gimple_fold_stmt_to_constant_1.
>
>> So you actually ran into a testcase that expected the warning?
>
> Several of them if I remember correctly...
>
>>> I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS ||
>>> TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict
>>> overflow), so I went with some kind of complement we use elsewhere.
>> 
>> I think I prefer to move things 1:1 (unless sth regresses) and fix bugs in 
>> the
>> fold-const.c variant as followup (possibly also adding testcases).
>
> Well, yes, but things do indeed regress quite regularly when moving things 
> 1:1 :-( At least unless we add a big (if (GENERIC) ...) around the 
> transformation.
>
>> IMHO -fno-strict-overflow needs to go.  It has very wary designed semantics
>> (ops neither wrap nor have undefined overflow).
>
> Maybe make it an alias for -fwrapv?

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10809 bytes --]

Index: trunk4/gcc/fold-const.c
===================================================================
--- trunk4/gcc/fold-const.c	(revision 235452)
+++ trunk4/gcc/fold-const.c	(working copy)
@@ -290,21 +290,21 @@ fold_undefer_and_ignore_overflow_warning
 
 bool
 fold_deferring_overflow_warnings_p (void)
 {
   return fold_deferring_overflow_warnings > 0;
 }
 
 /* This is called when we fold something based on the fact that signed
    overflow is undefined.  */
 
-static void
+void
 fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code wc)
 {
   if (fold_deferring_overflow_warnings > 0)
     {
       if (fold_deferred_overflow_warning == NULL
 	  || wc < fold_deferred_overflow_code)
 	{
 	  fold_deferred_overflow_warning = gmsgid;
 	  fold_deferred_overflow_code = wc;
 	}
@@ -8366,89 +8366,20 @@ fold_comparison (location_t loc, enum tr
 {
   const bool equality_code = (code == EQ_EXPR || code == NE_EXPR);
   tree arg0, arg1, tem;
 
   arg0 = op0;
   arg1 = op1;
 
   STRIP_SIGN_NOPS (arg0);
   STRIP_SIGN_NOPS (arg1);
 
-  /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
-  if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
-      && (equality_code
-	  || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg0))
-	      && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))))
-      && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
-      && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1))
-      && TREE_CODE (arg1) == INTEGER_CST
-      && !TREE_OVERFLOW (arg1))
-    {
-      const enum tree_code
-	reverse_op = TREE_CODE (arg0) == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR;
-      tree const1 = TREE_OPERAND (arg0, 1);
-      tree const2 = fold_convert_loc (loc, TREE_TYPE (const1), arg1);
-      tree variable = TREE_OPERAND (arg0, 0);
-      tree new_const = int_const_binop (reverse_op, const2, const1);
-
-      /* If the constant operation overflowed this can be
-	 simplified as a comparison against INT_MAX/INT_MIN.  */
-      if (TREE_OVERFLOW (new_const)
-	  && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
-	{
-	  int const1_sgn = tree_int_cst_sgn (const1);
-	  enum tree_code code2 = code;
-
-	  /* Get the sign of the constant on the lhs if the
-	     operation were VARIABLE + CONST1.  */
-	  if (TREE_CODE (arg0) == MINUS_EXPR)
-	    const1_sgn = -const1_sgn;
-
-	  /* The sign of the constant determines if we overflowed
-	     INT_MAX (const1_sgn == -1) or INT_MIN (const1_sgn == 1).
-	     Canonicalize to the INT_MIN overflow by swapping the comparison
-	     if necessary.  */
-	  if (const1_sgn == -1)
-	    code2 = swap_tree_comparison (code);
-
-	  /* We now can look at the canonicalized case
-	       VARIABLE + 1  CODE2  INT_MIN
-	     and decide on the result.  */
-	  switch (code2)
-	    {
-	    case EQ_EXPR:
-	    case LT_EXPR:
-	    case LE_EXPR:
-	      return
-		omit_one_operand_loc (loc, type, boolean_false_node, variable);
-
-	    case NE_EXPR:
-	    case GE_EXPR:
-	    case GT_EXPR:
-	      return
-		omit_one_operand_loc (loc, type, boolean_true_node, variable);
-
-	    default:
-	      gcc_unreachable ();
-	    }
-	}
-      else
-	{
-	  if (!equality_code)
-	    fold_overflow_warning ("assuming signed overflow does not occur "
-				   "when changing X +- C1 cmp C2 to "
-				   "X cmp C2 -+ C1",
-				   WARN_STRICT_OVERFLOW_COMPARISON);
-	  return fold_build2_loc (loc, code, type, variable, new_const);
-	}
-    }
-
   /* For comparisons of pointers we can decompose it to a compile time
      comparison of the base objects and the offsets into the object.
      This requires at least one operand being an ADDR_EXPR or a
      POINTER_PLUS_EXPR to do more than the operand_equal_p test below.  */
   if (POINTER_TYPE_P (TREE_TYPE (arg0))
       && (TREE_CODE (arg0) == ADDR_EXPR
 	  || TREE_CODE (arg1) == ADDR_EXPR
 	  || TREE_CODE (arg0) == POINTER_PLUS_EXPR
 	  || TREE_CODE (arg1) == POINTER_PLUS_EXPR))
     {
Index: trunk4/gcc/fold-const.h
===================================================================
--- trunk4/gcc/fold-const.h	(revision 235452)
+++ trunk4/gcc/fold-const.h	(working copy)
@@ -13,20 +13,22 @@ WARRANTY; without even the implied warra
 FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 for more details.
 
 You should have received a copy of the GNU General Public License
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #ifndef GCC_FOLD_CONST_H
 #define GCC_FOLD_CONST_H
 
+#include <flag-types.h>
+
 /* Non-zero if we are folding constants inside an initializer; zero
    otherwise.  */
 extern int folding_initializer;
 
 /* Convert between trees and native memory representation.  */
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
 extern tree native_interpret_expr (tree, const unsigned char *, int);
 
 /* Fold constants as much as possible in an expression.
    Returns the simplified expression.
@@ -79,20 +81,21 @@ extern bool fold_convertible_p (const_tr
    fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
 extern tree fold_convert_loc (location_t, tree, tree);
 extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree, tree);
 extern tree fold_ignored_result (tree);
 extern tree fold_abs_const (tree, tree);
 extern tree fold_indirect_ref_1 (location_t, tree, tree);
 extern void fold_defer_overflow_warnings (void);
 extern void fold_undefer_overflow_warnings (bool, const gimple *, int);
 extern void fold_undefer_and_ignore_overflow_warnings (void);
 extern bool fold_deferring_overflow_warnings_p (void);
+extern void fold_overflow_warning (const char*, enum warn_strict_overflow_code);
 extern int operand_equal_p (const_tree, const_tree, unsigned int);
 extern int multiple_of_p (tree, const_tree, const_tree);
 #define omit_one_operand(T1,T2,T3)\
    omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
 extern tree omit_one_operand_loc (location_t, tree, tree, tree);
 #define omit_two_operands(T1,T2,T3,T4)\
    omit_two_operands_loc (UNKNOWN_LOCATION, T1, T2, T3, T4)
 extern tree omit_two_operands_loc (location_t, tree, tree, tree, tree);
 #define invert_truthvalue(T)\
    invert_truthvalue_loc (UNKNOWN_LOCATION, T)
Index: trunk4/gcc/gimple-fold.c
===================================================================
--- trunk4/gcc/gimple-fold.c	(revision 235452)
+++ trunk4/gcc/gimple-fold.c	(working copy)
@@ -3519,20 +3519,21 @@ maybe_canonicalize_mem_ref_addr (tree *t
 
 /* Worker for both fold_stmt and fold_stmt_inplace.  The INPLACE argument
    distinguishes both cases.  */
 
 static bool
 fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize) (tree))
 {
   bool changed = false;
   gimple *stmt = gsi_stmt (*gsi);
   unsigned i;
+  fold_defer_overflow_warnings ();
 
   /* First do required canonicalization of [TARGET_]MEM_REF addresses
      after propagation.
      ???  This shouldn't be done in generic folding but in the
      propagation helpers which also know whether an address was
      propagated.
      Also canonicalize operand order.  */
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
@@ -3811,20 +3812,22 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
 	{
 	  tree new_lhs = maybe_fold_reference (lhs, true);
 	  if (new_lhs)
 	    {
 	      gimple_set_lhs (stmt, new_lhs);
 	      changed = true;
 	    }
 	}
     }
 
+  fold_undefer_overflow_warnings (changed && !gimple_no_warning_p (stmt),
+				  stmt, 0);
   return changed;
 }
 
 /* Valueziation callback that ends up not following SSA edges.  */
 
 tree
 no_follow_ssa_edges (tree)
 {
   return NULL_TREE;
 }
Index: trunk4/gcc/match.pd
===================================================================
--- trunk4/gcc/match.pd	(revision 235452)
+++ trunk4/gcc/match.pd	(working copy)
@@ -3099,10 +3099,54 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
  /* signbit(x) -> 0 if x is nonnegative.  */
  (SIGNBIT tree_expr_nonnegative_p@0)
  { integer_zero_node; })
 
 (simplify
  /* signbit(x) -> x<0 if x doesn't have signed zeros.  */
  (SIGNBIT @0)
  (if (!HONOR_SIGNED_ZEROS (@0))
   (convert (lt @0 { build_real (TREE_TYPE (@0), dconst0); }))))
+
+/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
+(for cmp (eq ne)
+ (for op (plus minus)
+      rop (minus plus)
+  (simplify
+   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
+   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
+	&& !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
+	&& !TYPE_OVERFLOW_TRAPS (TREE_TYPE (@0))
+	&& !TYPE_SATURATING (TREE_TYPE (@0)))
+    (with { tree res = int_const_binop (rop, @2, @1); }
+     (if (TREE_OVERFLOW (res))
+      { constant_boolean_node (cmp == NE_EXPR, type); }
+      (if (single_use (@3))
+       (cmp @0 { res; }))))))))
+(for cmp (lt le gt ge)
+ (for op (plus minus)
+      rop (minus plus)
+  (simplify
+   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
+   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
+	&& TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
+    (with { tree res = int_const_binop (rop, @2, @1); }
+     (if (TREE_OVERFLOW (res))
+      {
+	fold_overflow_warning (("assuming signed overflow does not occur "
+				"when simplifying conditional to constant"),
+			       WARN_STRICT_OVERFLOW_CONDITIONAL);
+        bool less = cmp == LE_EXPR || cmp == LT_EXPR;
+	/* wi::ges_p (@2, 0) should be sufficient for a signed type.  */
+	bool ovf_high = wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1)))
+			!= (op == MINUS_EXPR);
+	constant_boolean_node (less == ovf_high, type);
+      }
+      (if (single_use (@3))
+       (with
+	{
+	  fold_overflow_warning (("assuming signed overflow does not occur "
+				  "when changing X +- C1 cmp C2 to "
+				  "X cmp C2 -+ C1"),
+				 WARN_STRICT_OVERFLOW_COMPARISON);
+	}
+	(cmp @0 { res; })))))))))
Index: trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c
===================================================================
--- trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c	(revision 235452)
+++ trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c	(working copy)
@@ -16,15 +16,15 @@ void foo(int edx, int eax)
     }
   if (eax == 100)
     {
       if (-- edx == 0)
         afred[0] = 2;
     }
 }
  
 
 /* Verify that we did a forward propagation.  */
-/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
+/* { dg-final { scan-tree-dump-times "gimple_simplified" 1 "forwprop1"} } */
 
 /* After cddce we should have two IF statements remaining as the other
    two tests can be threaded.  */
 /* { dg-final { scan-tree-dump-times "if " 2 "cddce2"} } */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-26 20:28   ` Marc Glisse
  2016-04-27  5:34     ` Marc Glisse
@ 2016-04-27 11:52     ` Richard Biener
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Biener @ 2016-04-27 11:52 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Tue, Apr 26, 2016 at 10:28 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Tue, 26 Apr 2016, Richard Biener wrote:
>
>> On Sun, Apr 24, 2016 at 7:14 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> Hello,
>>>
>>> trying to move a first pattern from fold_comparison.
>>>
>>> I first tried without single_use. It brought the number of 'free' in
>>> g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2
>>> SMS
>>> (I don't think the generated code was worse, maybe even better, but I
>>> don't
>>> know ppc asm), broke Wstrict-overflow-18.c (the optimization moved from
>>> VRP
>>> to CCP if I remember correctly), and caused IVOPTS to make a mess in
>>> guality/pr54693-2.c (much longer code, and many <optimized> debug
>>> variables). If someone wants to drop the single_use, they can work on
>>> that
>>> after this patch is in.
>>>
>>> The conditions do not exactly match the ones in fold-const.c, but I guess
>>> they are close. The warning in the constant case was missing in
>>> fold_comparison, but present in VRP, so I had to add it not to regress.
>>>
>>> I don't think we were warning much from match.pd. I can't say that I am a
>>> big fan of those strict overflow warnings, but I expect people would
>>> complain if we just dropped the existing ones when moving the transforms
>>> to
>>> match.pd?
>>
>>
>> I just dropped them for patterns I moved (luckily we didn't have
>> testcases sofar ;))
>>
>> If we really want to warn from match.pd then you should do the
>> defer/undefer
>> stuff in fold_stmt itself (same condition I guess) and defer/undefer
>> without
>> warning in gimple_fold_stmt_to_constant_1.
>
>
> Moving it to fold_stmt_1 seems like a good idea, much better than putting it
> in forwprop. Looking at gimple_fold_stmt_to_constant_1 on the other hand, I
> have some concerns. If we do not warn for gimple_fold_stmt_to_constant_1, we
> are going to miss some warnings (I believe there is at least one testcase
> that will break, where VRP currently warns but CCP will come first). On the
> other hand, gimple_fold_stmt_to_constant_1 doesn't do much validation on its
> return value, each caller has their own idea of which results are
> acceptable, so it is only in the (many) callers that we can defer/undefer,
> or we might warn for transformations that we don't actually perform. CCP
> already has the defer/undefer calls around ccp_fold and thus
> gimple_fold_stmt_to_constant_1.

Yeah, the issue with gimple_fold_stmt_to_constant_1 is that it's usually
used in an iteration scheme and thus we may warn multiple times
and for transforms that don't end up being used...

>> So you actually ran into a testcase that expected the warning?
>
>
> Several of them if I remember correctly...

Ugh.

>>> I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS ||
>>> TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict
>>> overflow), so I went with some kind of complement we use elsewhere.
>>
>>
>> I think I prefer to move things 1:1 (unless sth regresses) and fix bugs in
>> the
>> fold-const.c variant as followup (possibly also adding testcases).
>
>
> Well, yes, but things do indeed regress quite regularly when moving things
> 1:1 :-( At least unless we add a big (if (GENERIC) ...) around the
> transformation.

Sure, just wanted clarification on what changes are just (obvious) bugfixes
and what are needed to not regress in the testsuite.

>> IMHO -fno-strict-overflow needs to go.  It has very wary designed
>> semantics
>> (ops neither wrap nor have undefined overflow).
>
>
> Maybe make it an alias for -fwrapv?

Yes, for example.

Richard.

>
> --
> Marc Glisse

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-27  5:34     ` Marc Glisse
@ 2016-04-27 12:02       ` Richard Biener
  2016-04-28 20:15         ` Marc Glisse
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2016-04-27 12:02 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Wed, Apr 27, 2016 at 7:34 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Here is the current patch (passed regtest), after moving defer/undefer from
> forwprop to fold_stmt_1. I am not sure if checking no_warning at the end of
> fold_stmt_1 is safe, or if I should save its value at the beginning of the
> function, in case some of the transformations clear it.

I think you need to check it on the original stmt, it is preserved only
if we re-use the old stmt memory.

>
> On Tue, 26 Apr 2016, Marc Glisse wrote:
>
>> On Tue, 26 Apr 2016, Richard Biener wrote:
>>
>>> On Sun, Apr 24, 2016 at 7:14 PM, Marc Glisse <marc.glisse@inria.fr>
>>> wrote:
>>>>
>>>> Hello,
>>>>
>>>> trying to move a first pattern from fold_comparison.
>>>>
>>>> I first tried without single_use. It brought the number of 'free' in
>>>> g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2
>>>> SMS
>>>> (I don't think the generated code was worse, maybe even better, but I
>>>> don't
>>>> know ppc asm), broke Wstrict-overflow-18.c (the optimization moved from
>>>> VRP
>>>> to CCP if I remember correctly), and caused IVOPTS to make a mess in
>>>> guality/pr54693-2.c (much longer code, and many <optimized> debug
>>>> variables). If someone wants to drop the single_use, they can work on
>>>> that
>>>> after this patch is in.
>>>>
>>>> The conditions do not exactly match the ones in fold-const.c, but I
>>>> guess
>>>> they are close. The warning in the constant case was missing in
>>>> fold_comparison, but present in VRP, so I had to add it not to regress.
>>>>
>>>> I don't think we were warning much from match.pd. I can't say that I am
>>>> a
>>>> big fan of those strict overflow warnings, but I expect people would
>>>> complain if we just dropped the existing ones when moving the transforms
>>>> to
>>>> match.pd?
>>>
>>>
>>> I just dropped them for patterns I moved (luckily we didn't have
>>> testcases sofar ;))
>>>
>>> If we really want to warn from match.pd then you should do the
>>> defer/undefer
>>> stuff in fold_stmt itself (same condition I guess) and defer/undefer
>>> without
>>> warning in gimple_fold_stmt_to_constant_1.
>>
>>
>> Moving it to fold_stmt_1 seems like a good idea, much better than putting
>> it in forwprop. Looking at gimple_fold_stmt_to_constant_1 on the other hand,
>> I have some concerns. If we do not warn for gimple_fold_stmt_to_constant_1,
>> we are going to miss some warnings (I believe there is at least one testcase
>> that will break, where VRP currently warns but CCP will come first). On the
>> other hand, gimple_fold_stmt_to_constant_1 doesn't do much validation on its
>> return value, each caller has their own idea of which results are
>> acceptable, so it is only in the (many) callers that we can defer/undefer,
>> or we might warn for transformations that we don't actually perform. CCP
>> already has the defer/undefer calls around ccp_fold and thus
>> gimple_fold_stmt_to_constant_1.
>>
>>> So you actually ran into a testcase that expected the warning?
>>
>>
>> Several of them if I remember correctly...
>>
>>>> I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS ||
>>>> TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict
>>>> overflow), so I went with some kind of complement we use elsewhere.
>>>
>>>
>>> I think I prefer to move things 1:1 (unless sth regresses) and fix bugs
>>> in the
>>> fold-const.c variant as followup (possibly also adding testcases).
>>
>>
>> Well, yes, but things do indeed regress quite regularly when moving things
>> 1:1 :-( At least unless we add a big (if (GENERIC) ...) around the
>> transformation.
>>
>>> IMHO -fno-strict-overflow needs to go.  It has very wary designed
>>> semantics
>>> (ops neither wrap nor have undefined overflow).
>>
>>
>> Maybe make it an alias for -fwrapv?
>
>
> --
> Marc Glisse
>
> Index: trunk4/gcc/fold-const.c
> ===================================================================
> --- trunk4/gcc/fold-const.c     (revision 235452)
> +++ trunk4/gcc/fold-const.c     (working copy)
> @@ -290,21 +290,21 @@ fold_undefer_and_ignore_overflow_warning
>
>  bool
>  fold_deferring_overflow_warnings_p (void)
>  {
>    return fold_deferring_overflow_warnings > 0;
>  }
>
>  /* This is called when we fold something based on the fact that signed
>     overflow is undefined.  */
>
> -static void
> +void
>  fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code
> wc)
>  {
>    if (fold_deferring_overflow_warnings > 0)
>      {
>        if (fold_deferred_overflow_warning == NULL
>           || wc < fold_deferred_overflow_code)
>         {
>           fold_deferred_overflow_warning = gmsgid;
>           fold_deferred_overflow_code = wc;
>         }
> @@ -8366,89 +8366,20 @@ fold_comparison (location_t loc, enum tr
>  {
>    const bool equality_code = (code == EQ_EXPR || code == NE_EXPR);
>    tree arg0, arg1, tem;
>
>    arg0 = op0;
>    arg1 = op1;
>
>    STRIP_SIGN_NOPS (arg0);
>    STRIP_SIGN_NOPS (arg1);
>
> -  /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.
> */
> -  if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
> -      && (equality_code
> -         || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg0))
> -             && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))))
> -      && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
> -      && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1))
> -      && TREE_CODE (arg1) == INTEGER_CST
> -      && !TREE_OVERFLOW (arg1))
> -    {
> -      const enum tree_code
> -       reverse_op = TREE_CODE (arg0) == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR;
> -      tree const1 = TREE_OPERAND (arg0, 1);
> -      tree const2 = fold_convert_loc (loc, TREE_TYPE (const1), arg1);
> -      tree variable = TREE_OPERAND (arg0, 0);
> -      tree new_const = int_const_binop (reverse_op, const2, const1);
> -
> -      /* If the constant operation overflowed this can be
> -        simplified as a comparison against INT_MAX/INT_MIN.  */
> -      if (TREE_OVERFLOW (new_const)
> -         && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
> -       {
> -         int const1_sgn = tree_int_cst_sgn (const1);
> -         enum tree_code code2 = code;
> -
> -         /* Get the sign of the constant on the lhs if the
> -            operation were VARIABLE + CONST1.  */
> -         if (TREE_CODE (arg0) == MINUS_EXPR)
> -           const1_sgn = -const1_sgn;
> -
> -         /* The sign of the constant determines if we overflowed
> -            INT_MAX (const1_sgn == -1) or INT_MIN (const1_sgn == 1).
> -            Canonicalize to the INT_MIN overflow by swapping the comparison
> -            if necessary.  */
> -         if (const1_sgn == -1)
> -           code2 = swap_tree_comparison (code);
> -
> -         /* We now can look at the canonicalized case
> -              VARIABLE + 1  CODE2  INT_MIN
> -            and decide on the result.  */
> -         switch (code2)
> -           {
> -           case EQ_EXPR:
> -           case LT_EXPR:
> -           case LE_EXPR:
> -             return
> -               omit_one_operand_loc (loc, type, boolean_false_node,
> variable);
> -
> -           case NE_EXPR:
> -           case GE_EXPR:
> -           case GT_EXPR:
> -             return
> -               omit_one_operand_loc (loc, type, boolean_true_node,
> variable);
> -
> -           default:
> -             gcc_unreachable ();
> -           }
> -       }
> -      else
> -       {
> -         if (!equality_code)
> -           fold_overflow_warning ("assuming signed overflow does not occur
> "
> -                                  "when changing X +- C1 cmp C2 to "
> -                                  "X cmp C2 -+ C1",
> -                                  WARN_STRICT_OVERFLOW_COMPARISON);
> -         return fold_build2_loc (loc, code, type, variable, new_const);
> -       }
> -    }
> -
>    /* For comparisons of pointers we can decompose it to a compile time
>       comparison of the base objects and the offsets into the object.
>       This requires at least one operand being an ADDR_EXPR or a
>       POINTER_PLUS_EXPR to do more than the operand_equal_p test below.  */
>    if (POINTER_TYPE_P (TREE_TYPE (arg0))
>        && (TREE_CODE (arg0) == ADDR_EXPR
>           || TREE_CODE (arg1) == ADDR_EXPR
>           || TREE_CODE (arg0) == POINTER_PLUS_EXPR
>           || TREE_CODE (arg1) == POINTER_PLUS_EXPR))
>      {
> Index: trunk4/gcc/fold-const.h
> ===================================================================
> --- trunk4/gcc/fold-const.h     (revision 235452)
> +++ trunk4/gcc/fold-const.h     (working copy)
> @@ -13,20 +13,22 @@ WARRANTY; without even the implied warra
>  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>  for more details.
>
>  You should have received a copy of the GNU General Public License
>  along with GCC; see the file COPYING3.  If not see
>  <http://www.gnu.org/licenses/>.  */
>
>  #ifndef GCC_FOLD_CONST_H
>  #define GCC_FOLD_CONST_H
>
> +#include <flag-types.h>
> +

I think the canonical way is to include options.h where you include
fold-const.h ...
(ick)

Doesn't the prototype serve as a forward declaration only and thus including
options.h from gimple-match-head.c is enough?

Otherwise the patch looks ok to me.

Thanks,
Richard.

>  /* Non-zero if we are folding constants inside an initializer; zero
>     otherwise.  */
>  extern int folding_initializer;
>
>  /* Convert between trees and native memory representation.  */
>  extern int native_encode_expr (const_tree, unsigned char *, int, int off =
> -1);
>  extern tree native_interpret_expr (tree, const unsigned char *, int);
>
>  /* Fold constants as much as possible in an expression.
>     Returns the simplified expression.
> @@ -79,20 +81,21 @@ extern bool fold_convertible_p (const_tr
>     fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
>  extern tree fold_convert_loc (location_t, tree, tree);
>  extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree,
> tree);
>  extern tree fold_ignored_result (tree);
>  extern tree fold_abs_const (tree, tree);
>  extern tree fold_indirect_ref_1 (location_t, tree, tree);
>  extern void fold_defer_overflow_warnings (void);
>  extern void fold_undefer_overflow_warnings (bool, const gimple *, int);
>  extern void fold_undefer_and_ignore_overflow_warnings (void);
>  extern bool fold_deferring_overflow_warnings_p (void);
> +extern void fold_overflow_warning (const char*, enum
> warn_strict_overflow_code);
>  extern int operand_equal_p (const_tree, const_tree, unsigned int);
>  extern int multiple_of_p (tree, const_tree, const_tree);
>  #define omit_one_operand(T1,T2,T3)\
>     omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
>  extern tree omit_one_operand_loc (location_t, tree, tree, tree);
>  #define omit_two_operands(T1,T2,T3,T4)\
>     omit_two_operands_loc (UNKNOWN_LOCATION, T1, T2, T3, T4)
>  extern tree omit_two_operands_loc (location_t, tree, tree, tree, tree);
>  #define invert_truthvalue(T)\
>     invert_truthvalue_loc (UNKNOWN_LOCATION, T)
> Index: trunk4/gcc/gimple-fold.c
> ===================================================================
> --- trunk4/gcc/gimple-fold.c    (revision 235452)
> +++ trunk4/gcc/gimple-fold.c    (working copy)
> @@ -3519,20 +3519,21 @@ maybe_canonicalize_mem_ref_addr (tree *t
>
>  /* Worker for both fold_stmt and fold_stmt_inplace.  The INPLACE argument
>     distinguishes both cases.  */
>
>  static bool
>  fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize)
> (tree))
>  {
>    bool changed = false;
>    gimple *stmt = gsi_stmt (*gsi);
>    unsigned i;
> +  fold_defer_overflow_warnings ();
>
>    /* First do required canonicalization of [TARGET_]MEM_REF addresses
>       after propagation.
>       ???  This shouldn't be done in generic folding but in the
>       propagation helpers which also know whether an address was
>       propagated.
>       Also canonicalize operand order.  */
>    switch (gimple_code (stmt))
>      {
>      case GIMPLE_ASSIGN:
> @@ -3811,20 +3812,22 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>         {
>           tree new_lhs = maybe_fold_reference (lhs, true);
>           if (new_lhs)
>             {
>               gimple_set_lhs (stmt, new_lhs);
>               changed = true;
>             }
>         }
>      }
>
> +  fold_undefer_overflow_warnings (changed && !gimple_no_warning_p (stmt),
> +                                 stmt, 0);
>    return changed;
>  }
>
>  /* Valueziation callback that ends up not following SSA edges.  */
>
>  tree
>  no_follow_ssa_edges (tree)
>  {
>    return NULL_TREE;
>  }
> Index: trunk4/gcc/match.pd
> ===================================================================
> --- trunk4/gcc/match.pd (revision 235452)
> +++ trunk4/gcc/match.pd (working copy)
> @@ -3099,10 +3099,54 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (simplify
>   /* signbit(x) -> 0 if x is nonnegative.  */
>   (SIGNBIT tree_expr_nonnegative_p@0)
>   { integer_zero_node; })
>
>  (simplify
>   /* signbit(x) -> x<0 if x doesn't have signed zeros.  */
>   (SIGNBIT @0)
>   (if (!HONOR_SIGNED_ZEROS (@0))
>    (convert (lt @0 { build_real (TREE_TYPE (@0), dconst0); }))))
> +
> +/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
> +(for cmp (eq ne)
> + (for op (plus minus)
> +      rop (minus plus)
> +  (simplify
> +   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
> +   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
> +       && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
> +       && !TYPE_OVERFLOW_TRAPS (TREE_TYPE (@0))
> +       && !TYPE_SATURATING (TREE_TYPE (@0)))
> +    (with { tree res = int_const_binop (rop, @2, @1); }
> +     (if (TREE_OVERFLOW (res))
> +      { constant_boolean_node (cmp == NE_EXPR, type); }
> +      (if (single_use (@3))
> +       (cmp @0 { res; }))))))))
> +(for cmp (lt le gt ge)
> + (for op (plus minus)
> +      rop (minus plus)
> +  (simplify
> +   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
> +   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
> +       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
> +    (with { tree res = int_const_binop (rop, @2, @1); }
> +     (if (TREE_OVERFLOW (res))
> +      {
> +       fold_overflow_warning (("assuming signed overflow does not occur "
> +                               "when simplifying conditional to constant"),
> +                              WARN_STRICT_OVERFLOW_CONDITIONAL);
> +        bool less = cmp == LE_EXPR || cmp == LT_EXPR;
> +       /* wi::ges_p (@2, 0) should be sufficient for a signed type.  */
> +       bool ovf_high = wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1)))
> +                       != (op == MINUS_EXPR);
> +       constant_boolean_node (less == ovf_high, type);
> +      }
> +      (if (single_use (@3))
> +       (with
> +       {
> +         fold_overflow_warning (("assuming signed overflow does not occur "
> +                                 "when changing X +- C1 cmp C2 to "
> +                                 "X cmp C2 -+ C1"),
> +                                WARN_STRICT_OVERFLOW_COMPARISON);
> +       }
> +       (cmp @0 { res; })))))))))
> Index: trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c
> ===================================================================
> --- trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c   (revision 235452)
> +++ trunk4/gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c   (working copy)
> @@ -16,15 +16,15 @@ void foo(int edx, int eax)
>      }
>    if (eax == 100)
>      {
>        if (-- edx == 0)
>          afred[0] = 2;
>      }
>  }
>
>
>  /* Verify that we did a forward propagation.  */
> -/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
> +/* { dg-final { scan-tree-dump-times "gimple_simplified" 1 "forwprop1"} }
> */
>
>  /* After cddce we should have two IF statements remaining as the other
>     two tests can be threaded.  */
>  /* { dg-final { scan-tree-dump-times "if " 2 "cddce2"} } */
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-27 12:02       ` Richard Biener
@ 2016-04-28 20:15         ` Marc Glisse
  2016-04-29  8:08           ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Glisse @ 2016-04-28 20:15 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Wed, 27 Apr 2016, Richard Biener wrote:

>> --- trunk4/gcc/fold-const.h     (revision 235452)
>> +++ trunk4/gcc/fold-const.h     (working copy)
>> @@ -13,20 +13,22 @@ WARRANTY; without even the implied warra
>>  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>>  for more details.
>>
>>  You should have received a copy of the GNU General Public License
>>  along with GCC; see the file COPYING3.  If not see
>>  <http://www.gnu.org/licenses/>.  */
>>
>>  #ifndef GCC_FOLD_CONST_H
>>  #define GCC_FOLD_CONST_H
>>
>> +#include <flag-types.h>
>> +
>
> I think the canonical way is to include options.h where you include
> fold-const.h ...
> (ick)
>
> Doesn't the prototype serve as a forward declaration only and thus including
> options.h from gimple-match-head.c is enough?

Doesn't look like it. If I remove this include, I get build failures for
a large part of the C front-end (through c-family/c-common.h) and
tree-ssa-scopedtables.c. Including options.h in those 2 files seems to
work (I didn't check if all the files in config/ that include
fold-const.h also indirectly include options.h). If you really think
that's better, I'll do it...

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-28 20:15         ` Marc Glisse
@ 2016-04-29  8:08           ` Richard Biener
  2016-04-29 17:36             ` Marc Glisse
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2016-04-29  8:08 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Thu, Apr 28, 2016 at 10:15 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Wed, 27 Apr 2016, Richard Biener wrote:
>
>>> --- trunk4/gcc/fold-const.h     (revision 235452)
>>> +++ trunk4/gcc/fold-const.h     (working copy)
>>> @@ -13,20 +13,22 @@ WARRANTY; without even the implied warra
>>>  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>>>  for more details.
>>>
>>>  You should have received a copy of the GNU General Public License
>>>  along with GCC; see the file COPYING3.  If not see
>>>  <http://www.gnu.org/licenses/>.  */
>>>
>>>  #ifndef GCC_FOLD_CONST_H
>>>  #define GCC_FOLD_CONST_H
>>>
>>> +#include <flag-types.h>
>>> +
>>
>>
>> I think the canonical way is to include options.h where you include
>> fold-const.h ...
>> (ick)
>>
>> Doesn't the prototype serve as a forward declaration only and thus
>> including
>> options.h from gimple-match-head.c is enough?
>
>
> Doesn't look like it. If I remove this include, I get build failures for
> a large part of the C front-end (through c-family/c-common.h) and
> tree-ssa-scopedtables.c. Including options.h in those 2 files seems to
> work (I didn't check if all the files in config/ that include
> fold-const.h also indirectly include options.h). If you really think
> that's better, I'll do it...

Another option is to move the enum declaration from flag-types.h to
coretypes.h.  I think I like that best.

Richard.

> --
> Marc Glisse

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-29  8:08           ` Richard Biener
@ 2016-04-29 17:36             ` Marc Glisse
  2016-05-02  8:29               ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Glisse @ 2016-04-29 17:36 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 686 bytes --]

On Fri, 29 Apr 2016, Richard Biener wrote:

> Another option is to move the enum declaration from flag-types.h to
> coretypes.h.  I think I like that best.

This works.

2016-05-02  Marc Glisse  <marc.glisse@inria.fr>

gcc/
 	* flag-types.h (enum warn_strict_overflow_code): Move ...
 	* coretypes.h: ... here.
 	* fold-const.h (fold_overflow_warning): Declare.
 	* fold-const.c (fold_overflow_warning): Make non-static.
 	(fold_comparison): Move the transformation of X +- C1 CMP C2
 	into X CMP C2 -+ C1 ...
 	* match.pd: ... here.
 	* gimple-fold.c (fold_stmt_1): Protect with
 	fold_defer_overflow_warnings.

gcc/testsuite/
 	* gcc.dg/tree-ssa/20040305-1.c: Adjust.

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 14112 bytes --]

Index: gcc/coretypes.h
===================================================================
--- gcc/coretypes.h	(revision 235644)
+++ gcc/coretypes.h	(working copy)
@@ -215,20 +215,44 @@ enum optimization_type {
 /* Possible initialization status of a variable.   When requested
    by the user, this information is tracked and recorded in the DWARF
    debug information, along with the variable's location.  */
 enum var_init_status
 {
   VAR_INIT_STATUS_UNKNOWN,
   VAR_INIT_STATUS_UNINITIALIZED,
   VAR_INIT_STATUS_INITIALIZED
 };
 
+/* Names for the different levels of -Wstrict-overflow=N.  The numeric
+   values here correspond to N.  */
+enum warn_strict_overflow_code
+{
+  /* Overflow warning that should be issued with -Wall: a questionable
+     construct that is easy to avoid even when using macros.  Example:
+     folding (x + CONSTANT > x) to 1.  */
+  WARN_STRICT_OVERFLOW_ALL = 1,
+  /* Overflow warning about folding a comparison to a constant because
+     of undefined signed overflow, other than cases covered by
+     WARN_STRICT_OVERFLOW_ALL.  Example: folding (abs (x) >= 0) to 1
+     (this is false when x == INT_MIN).  */
+  WARN_STRICT_OVERFLOW_CONDITIONAL = 2,
+  /* Overflow warning about changes to comparisons other than folding
+     them to a constant.  Example: folding (x + 1 > 1) to (x > 0).  */
+  WARN_STRICT_OVERFLOW_COMPARISON = 3,
+  /* Overflow warnings not covered by the above cases.  Example:
+     folding ((x * 10) / 5) to (x * 2).  */
+  WARN_STRICT_OVERFLOW_MISC = 4,
+  /* Overflow warnings about reducing magnitude of constants in
+     comparison.  Example: folding (x + 2 > y) to (x + 1 >= y).  */
+  WARN_STRICT_OVERFLOW_MAGNITUDE = 5
+};
+
 /* The type of an alias set.  Code currently assumes that variables of
    this type can take the values 0 (the alias set which aliases
    everything) and -1 (sometimes indicating that the alias set is
    unknown, sometimes indicating a memory barrier) and -2 (indicating
    that the alias set should be set to a unique value but has not been
    set yet).  */
 typedef int alias_set_type;
 
 struct edge_def;
 typedef struct edge_def *edge;
Index: gcc/flag-types.h
===================================================================
--- gcc/flag-types.h	(revision 235644)
+++ gcc/flag-types.h	(working copy)
@@ -171,44 +171,20 @@ enum stack_check_type
   /* Check the stack and rely on the target configuration files to
      check the static frame of functions, i.e. use the generic
      mechanism only for dynamic stack allocations.  */
   STATIC_BUILTIN_STACK_CHECK,
 
   /* Check the stack and entirely rely on the target configuration
      files, i.e. do not use the generic mechanism at all.  */
   FULL_BUILTIN_STACK_CHECK
 };
 
-/* Names for the different levels of -Wstrict-overflow=N.  The numeric
-   values here correspond to N.  */
-enum warn_strict_overflow_code
-{
-  /* Overflow warning that should be issued with -Wall: a questionable
-     construct that is easy to avoid even when using macros.  Example:
-     folding (x + CONSTANT > x) to 1.  */
-  WARN_STRICT_OVERFLOW_ALL = 1,
-  /* Overflow warning about folding a comparison to a constant because
-     of undefined signed overflow, other than cases covered by
-     WARN_STRICT_OVERFLOW_ALL.  Example: folding (abs (x) >= 0) to 1
-     (this is false when x == INT_MIN).  */
-  WARN_STRICT_OVERFLOW_CONDITIONAL = 2,
-  /* Overflow warning about changes to comparisons other than folding
-     them to a constant.  Example: folding (x + 1 > 1) to (x > 0).  */
-  WARN_STRICT_OVERFLOW_COMPARISON = 3,
-  /* Overflow warnings not covered by the above cases.  Example:
-     folding ((x * 10) / 5) to (x * 2).  */
-  WARN_STRICT_OVERFLOW_MISC = 4,
-  /* Overflow warnings about reducing magnitude of constants in
-     comparison.  Example: folding (x + 2 > y) to (x + 1 >= y).  */
-  WARN_STRICT_OVERFLOW_MAGNITUDE = 5
-};
-
 /* Floating-point contraction mode.  */
 enum fp_contract_mode {
   FP_CONTRACT_OFF = 0,
   FP_CONTRACT_ON = 1,
   FP_CONTRACT_FAST = 2
 };
 
 /* Scalar storage order kind.  */
 enum scalar_storage_order_kind {
   SSO_NATIVE = 0,
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 235644)
+++ gcc/fold-const.c	(working copy)
@@ -290,21 +290,21 @@ fold_undefer_and_ignore_overflow_warning
 
 bool
 fold_deferring_overflow_warnings_p (void)
 {
   return fold_deferring_overflow_warnings > 0;
 }
 
 /* This is called when we fold something based on the fact that signed
    overflow is undefined.  */
 
-static void
+void
 fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code wc)
 {
   if (fold_deferring_overflow_warnings > 0)
     {
       if (fold_deferred_overflow_warning == NULL
 	  || wc < fold_deferred_overflow_code)
 	{
 	  fold_deferred_overflow_warning = gmsgid;
 	  fold_deferred_overflow_code = wc;
 	}
@@ -8388,89 +8388,20 @@ fold_comparison (location_t loc, enum tr
 {
   const bool equality_code = (code == EQ_EXPR || code == NE_EXPR);
   tree arg0, arg1, tem;
 
   arg0 = op0;
   arg1 = op1;
 
   STRIP_SIGN_NOPS (arg0);
   STRIP_SIGN_NOPS (arg1);
 
-  /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
-  if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
-      && (equality_code
-	  || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg0))
-	      && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))))
-      && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
-      && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1))
-      && TREE_CODE (arg1) == INTEGER_CST
-      && !TREE_OVERFLOW (arg1))
-    {
-      const enum tree_code
-	reverse_op = TREE_CODE (arg0) == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR;
-      tree const1 = TREE_OPERAND (arg0, 1);
-      tree const2 = fold_convert_loc (loc, TREE_TYPE (const1), arg1);
-      tree variable = TREE_OPERAND (arg0, 0);
-      tree new_const = int_const_binop (reverse_op, const2, const1);
-
-      /* If the constant operation overflowed this can be
-	 simplified as a comparison against INT_MAX/INT_MIN.  */
-      if (TREE_OVERFLOW (new_const)
-	  && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
-	{
-	  int const1_sgn = tree_int_cst_sgn (const1);
-	  enum tree_code code2 = code;
-
-	  /* Get the sign of the constant on the lhs if the
-	     operation were VARIABLE + CONST1.  */
-	  if (TREE_CODE (arg0) == MINUS_EXPR)
-	    const1_sgn = -const1_sgn;
-
-	  /* The sign of the constant determines if we overflowed
-	     INT_MAX (const1_sgn == -1) or INT_MIN (const1_sgn == 1).
-	     Canonicalize to the INT_MIN overflow by swapping the comparison
-	     if necessary.  */
-	  if (const1_sgn == -1)
-	    code2 = swap_tree_comparison (code);
-
-	  /* We now can look at the canonicalized case
-	       VARIABLE + 1  CODE2  INT_MIN
-	     and decide on the result.  */
-	  switch (code2)
-	    {
-	    case EQ_EXPR:
-	    case LT_EXPR:
-	    case LE_EXPR:
-	      return
-		omit_one_operand_loc (loc, type, boolean_false_node, variable);
-
-	    case NE_EXPR:
-	    case GE_EXPR:
-	    case GT_EXPR:
-	      return
-		omit_one_operand_loc (loc, type, boolean_true_node, variable);
-
-	    default:
-	      gcc_unreachable ();
-	    }
-	}
-      else
-	{
-	  if (!equality_code)
-	    fold_overflow_warning ("assuming signed overflow does not occur "
-				   "when changing X +- C1 cmp C2 to "
-				   "X cmp C2 -+ C1",
-				   WARN_STRICT_OVERFLOW_COMPARISON);
-	  return fold_build2_loc (loc, code, type, variable, new_const);
-	}
-    }
-
   /* For comparisons of pointers we can decompose it to a compile time
      comparison of the base objects and the offsets into the object.
      This requires at least one operand being an ADDR_EXPR or a
      POINTER_PLUS_EXPR to do more than the operand_equal_p test below.  */
   if (POINTER_TYPE_P (TREE_TYPE (arg0))
       && (TREE_CODE (arg0) == ADDR_EXPR
 	  || TREE_CODE (arg1) == ADDR_EXPR
 	  || TREE_CODE (arg0) == POINTER_PLUS_EXPR
 	  || TREE_CODE (arg1) == POINTER_PLUS_EXPR))
     {
Index: gcc/fold-const.h
===================================================================
--- gcc/fold-const.h	(revision 235644)
+++ gcc/fold-const.h	(working copy)
@@ -79,20 +79,21 @@ extern bool fold_convertible_p (const_tr
    fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
 extern tree fold_convert_loc (location_t, tree, tree);
 extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree, tree);
 extern tree fold_ignored_result (tree);
 extern tree fold_abs_const (tree, tree);
 extern tree fold_indirect_ref_1 (location_t, tree, tree);
 extern void fold_defer_overflow_warnings (void);
 extern void fold_undefer_overflow_warnings (bool, const gimple *, int);
 extern void fold_undefer_and_ignore_overflow_warnings (void);
 extern bool fold_deferring_overflow_warnings_p (void);
+extern void fold_overflow_warning (const char*, enum warn_strict_overflow_code);
 extern int operand_equal_p (const_tree, const_tree, unsigned int);
 extern int multiple_of_p (tree, const_tree, const_tree);
 #define omit_one_operand(T1,T2,T3)\
    omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
 extern tree omit_one_operand_loc (location_t, tree, tree, tree);
 #define omit_two_operands(T1,T2,T3,T4)\
    omit_two_operands_loc (UNKNOWN_LOCATION, T1, T2, T3, T4)
 extern tree omit_two_operands_loc (location_t, tree, tree, tree, tree);
 #define invert_truthvalue(T)\
    invert_truthvalue_loc (UNKNOWN_LOCATION, T)
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 235644)
+++ gcc/gimple-fold.c	(working copy)
@@ -3518,21 +3518,23 @@ maybe_canonicalize_mem_ref_addr (tree *t
 }
 
 /* Worker for both fold_stmt and fold_stmt_inplace.  The INPLACE argument
    distinguishes both cases.  */
 
 static bool
 fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize) (tree))
 {
   bool changed = false;
   gimple *stmt = gsi_stmt (*gsi);
+  bool nowarning = gimple_no_warning_p (stmt);
   unsigned i;
+  fold_defer_overflow_warnings ();
 
   /* First do required canonicalization of [TARGET_]MEM_REF addresses
      after propagation.
      ???  This shouldn't be done in generic folding but in the
      propagation helpers which also know whether an address was
      propagated.
      Also canonicalize operand order.  */
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
@@ -3811,20 +3813,21 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
 	{
 	  tree new_lhs = maybe_fold_reference (lhs, true);
 	  if (new_lhs)
 	    {
 	      gimple_set_lhs (stmt, new_lhs);
 	      changed = true;
 	    }
 	}
     }
 
+  fold_undefer_overflow_warnings (changed && !nowarning, stmt, 0);
   return changed;
 }
 
 /* Valueziation callback that ends up not following SSA edges.  */
 
 tree
 no_follow_ssa_edges (tree)
 {
   return NULL_TREE;
 }
Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 235644)
+++ gcc/match.pd	(working copy)
@@ -3179,10 +3179,54 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
  /* signbit(x) -> 0 if x is nonnegative.  */
  (SIGNBIT tree_expr_nonnegative_p@0)
  { integer_zero_node; })
 
 (simplify
  /* signbit(x) -> x<0 if x doesn't have signed zeros.  */
  (SIGNBIT @0)
  (if (!HONOR_SIGNED_ZEROS (@0))
   (convert (lt @0 { build_real (TREE_TYPE (@0), dconst0); }))))
+
+/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
+(for cmp (eq ne)
+ (for op (plus minus)
+      rop (minus plus)
+  (simplify
+   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
+   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
+	&& !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
+	&& !TYPE_OVERFLOW_TRAPS (TREE_TYPE (@0))
+	&& !TYPE_SATURATING (TREE_TYPE (@0)))
+    (with { tree res = int_const_binop (rop, @2, @1); }
+     (if (TREE_OVERFLOW (res))
+      { constant_boolean_node (cmp == NE_EXPR, type); }
+      (if (single_use (@3))
+       (cmp @0 { res; }))))))))
+(for cmp (lt le gt ge)
+ (for op (plus minus)
+      rop (minus plus)
+  (simplify
+   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
+   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
+	&& TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
+    (with { tree res = int_const_binop (rop, @2, @1); }
+     (if (TREE_OVERFLOW (res))
+      {
+	fold_overflow_warning (("assuming signed overflow does not occur "
+				"when simplifying conditional to constant"),
+			       WARN_STRICT_OVERFLOW_CONDITIONAL);
+        bool less = cmp == LE_EXPR || cmp == LT_EXPR;
+	/* wi::ges_p (@2, 0) should be sufficient for a signed type.  */
+	bool ovf_high = wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1)))
+			!= (op == MINUS_EXPR);
+	constant_boolean_node (less == ovf_high, type);
+      }
+      (if (single_use (@3))
+       (with
+	{
+	  fold_overflow_warning (("assuming signed overflow does not occur "
+				  "when changing X +- C1 cmp C2 to "
+				  "X cmp C2 -+ C1"),
+				 WARN_STRICT_OVERFLOW_COMPARISON);
+	}
+	(cmp @0 { res; })))))))))
Index: gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c	(revision 235644)
+++ gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c	(working copy)
@@ -16,15 +16,15 @@ void foo(int edx, int eax)
     }
   if (eax == 100)
     {
       if (-- edx == 0)
         afred[0] = 2;
     }
 }
  
 
 /* Verify that we did a forward propagation.  */
-/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
+/* { dg-final { scan-tree-dump-times "gimple_simplified" 1 "forwprop1"} } */
 
 /* After cddce we should have two IF statements remaining as the other
    two tests can be threaded.  */
 /* { dg-final { scan-tree-dump-times "if " 2 "cddce2"} } */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd
  2016-04-29 17:36             ` Marc Glisse
@ 2016-05-02  8:29               ` Richard Biener
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Biener @ 2016-05-02  8:29 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Fri, Apr 29, 2016 at 7:36 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Fri, 29 Apr 2016, Richard Biener wrote:
>
>> Another option is to move the enum declaration from flag-types.h to
>> coretypes.h.  I think I like that best.
>
>
> This works.

Ok.

Thanks,
Richard.

> 2016-05-02  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/
>         * flag-types.h (enum warn_strict_overflow_code): Move ...
>         * coretypes.h: ... here.
>         * fold-const.h (fold_overflow_warning): Declare.
>         * fold-const.c (fold_overflow_warning): Make non-static.
>         (fold_comparison): Move the transformation of X +- C1 CMP C2
>         into X CMP C2 -+ C1 ...
>         * match.pd: ... here.
>         * gimple-fold.c (fold_stmt_1): Protect with
>
>         fold_defer_overflow_warnings.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/20040305-1.c: Adjust.
>
> --
> Marc Glisse
>
> Index: gcc/coretypes.h
> ===================================================================
> --- gcc/coretypes.h     (revision 235644)
> +++ gcc/coretypes.h     (working copy)
> @@ -215,20 +215,44 @@ enum optimization_type {
>  /* Possible initialization status of a variable.   When requested
>     by the user, this information is tracked and recorded in the DWARF
>     debug information, along with the variable's location.  */
>  enum var_init_status
>  {
>    VAR_INIT_STATUS_UNKNOWN,
>    VAR_INIT_STATUS_UNINITIALIZED,
>    VAR_INIT_STATUS_INITIALIZED
>  };
>
> +/* Names for the different levels of -Wstrict-overflow=N.  The numeric
> +   values here correspond to N.  */
> +enum warn_strict_overflow_code
> +{
> +  /* Overflow warning that should be issued with -Wall: a questionable
> +     construct that is easy to avoid even when using macros.  Example:
> +     folding (x + CONSTANT > x) to 1.  */
> +  WARN_STRICT_OVERFLOW_ALL = 1,
> +  /* Overflow warning about folding a comparison to a constant because
> +     of undefined signed overflow, other than cases covered by
> +     WARN_STRICT_OVERFLOW_ALL.  Example: folding (abs (x) >= 0) to 1
> +     (this is false when x == INT_MIN).  */
> +  WARN_STRICT_OVERFLOW_CONDITIONAL = 2,
> +  /* Overflow warning about changes to comparisons other than folding
> +     them to a constant.  Example: folding (x + 1 > 1) to (x > 0).  */
> +  WARN_STRICT_OVERFLOW_COMPARISON = 3,
> +  /* Overflow warnings not covered by the above cases.  Example:
> +     folding ((x * 10) / 5) to (x * 2).  */
> +  WARN_STRICT_OVERFLOW_MISC = 4,
> +  /* Overflow warnings about reducing magnitude of constants in
> +     comparison.  Example: folding (x + 2 > y) to (x + 1 >= y).  */
> +  WARN_STRICT_OVERFLOW_MAGNITUDE = 5
> +};
> +
>  /* The type of an alias set.  Code currently assumes that variables of
>     this type can take the values 0 (the alias set which aliases
>     everything) and -1 (sometimes indicating that the alias set is
>     unknown, sometimes indicating a memory barrier) and -2 (indicating
>     that the alias set should be set to a unique value but has not been
>     set yet).  */
>  typedef int alias_set_type;
>
>  struct edge_def;
>  typedef struct edge_def *edge;
> Index: gcc/flag-types.h
> ===================================================================
> --- gcc/flag-types.h    (revision 235644)
> +++ gcc/flag-types.h    (working copy)
> @@ -171,44 +171,20 @@ enum stack_check_type
>    /* Check the stack and rely on the target configuration files to
>       check the static frame of functions, i.e. use the generic
>       mechanism only for dynamic stack allocations.  */
>    STATIC_BUILTIN_STACK_CHECK,
>
>    /* Check the stack and entirely rely on the target configuration
>       files, i.e. do not use the generic mechanism at all.  */
>    FULL_BUILTIN_STACK_CHECK
>  };
>
> -/* Names for the different levels of -Wstrict-overflow=N.  The numeric
> -   values here correspond to N.  */
> -enum warn_strict_overflow_code
> -{
> -  /* Overflow warning that should be issued with -Wall: a questionable
> -     construct that is easy to avoid even when using macros.  Example:
> -     folding (x + CONSTANT > x) to 1.  */
> -  WARN_STRICT_OVERFLOW_ALL = 1,
> -  /* Overflow warning about folding a comparison to a constant because
> -     of undefined signed overflow, other than cases covered by
> -     WARN_STRICT_OVERFLOW_ALL.  Example: folding (abs (x) >= 0) to 1
> -     (this is false when x == INT_MIN).  */
> -  WARN_STRICT_OVERFLOW_CONDITIONAL = 2,
> -  /* Overflow warning about changes to comparisons other than folding
> -     them to a constant.  Example: folding (x + 1 > 1) to (x > 0).  */
> -  WARN_STRICT_OVERFLOW_COMPARISON = 3,
> -  /* Overflow warnings not covered by the above cases.  Example:
> -     folding ((x * 10) / 5) to (x * 2).  */
> -  WARN_STRICT_OVERFLOW_MISC = 4,
> -  /* Overflow warnings about reducing magnitude of constants in
> -     comparison.  Example: folding (x + 2 > y) to (x + 1 >= y).  */
> -  WARN_STRICT_OVERFLOW_MAGNITUDE = 5
> -};
> -
>  /* Floating-point contraction mode.  */
>  enum fp_contract_mode {
>    FP_CONTRACT_OFF = 0,
>    FP_CONTRACT_ON = 1,
>    FP_CONTRACT_FAST = 2
>  };
>
>  /* Scalar storage order kind.  */
>  enum scalar_storage_order_kind {
>    SSO_NATIVE = 0,
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c    (revision 235644)
> +++ gcc/fold-const.c    (working copy)
> @@ -290,21 +290,21 @@ fold_undefer_and_ignore_overflow_warning
>
>  bool
>  fold_deferring_overflow_warnings_p (void)
>  {
>    return fold_deferring_overflow_warnings > 0;
>  }
>
>  /* This is called when we fold something based on the fact that signed
>     overflow is undefined.  */
>
> -static void
> +void
>  fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code
> wc)
>  {
>    if (fold_deferring_overflow_warnings > 0)
>      {
>        if (fold_deferred_overflow_warning == NULL
>           || wc < fold_deferred_overflow_code)
>         {
>           fold_deferred_overflow_warning = gmsgid;
>           fold_deferred_overflow_code = wc;
>         }
> @@ -8388,89 +8388,20 @@ fold_comparison (location_t loc, enum tr
>  {
>    const bool equality_code = (code == EQ_EXPR || code == NE_EXPR);
>    tree arg0, arg1, tem;
>
>    arg0 = op0;
>    arg1 = op1;
>
>    STRIP_SIGN_NOPS (arg0);
>    STRIP_SIGN_NOPS (arg1);
>
> -  /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.
> */
> -  if ((TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
> -      && (equality_code
> -         || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg0))
> -             && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))))
> -      && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
> -      && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1))
> -      && TREE_CODE (arg1) == INTEGER_CST
> -      && !TREE_OVERFLOW (arg1))
> -    {
> -      const enum tree_code
> -       reverse_op = TREE_CODE (arg0) == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR;
> -      tree const1 = TREE_OPERAND (arg0, 1);
> -      tree const2 = fold_convert_loc (loc, TREE_TYPE (const1), arg1);
> -      tree variable = TREE_OPERAND (arg0, 0);
> -      tree new_const = int_const_binop (reverse_op, const2, const1);
> -
> -      /* If the constant operation overflowed this can be
> -        simplified as a comparison against INT_MAX/INT_MIN.  */
> -      if (TREE_OVERFLOW (new_const)
> -         && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
> -       {
> -         int const1_sgn = tree_int_cst_sgn (const1);
> -         enum tree_code code2 = code;
> -
> -         /* Get the sign of the constant on the lhs if the
> -            operation were VARIABLE + CONST1.  */
> -         if (TREE_CODE (arg0) == MINUS_EXPR)
> -           const1_sgn = -const1_sgn;
> -
> -         /* The sign of the constant determines if we overflowed
> -            INT_MAX (const1_sgn == -1) or INT_MIN (const1_sgn == 1).
> -            Canonicalize to the INT_MIN overflow by swapping the comparison
> -            if necessary.  */
> -         if (const1_sgn == -1)
> -           code2 = swap_tree_comparison (code);
> -
> -         /* We now can look at the canonicalized case
> -              VARIABLE + 1  CODE2  INT_MIN
> -            and decide on the result.  */
> -         switch (code2)
> -           {
> -           case EQ_EXPR:
> -           case LT_EXPR:
> -           case LE_EXPR:
> -             return
> -               omit_one_operand_loc (loc, type, boolean_false_node,
> variable);
> -
> -           case NE_EXPR:
> -           case GE_EXPR:
> -           case GT_EXPR:
> -             return
> -               omit_one_operand_loc (loc, type, boolean_true_node,
> variable);
> -
> -           default:
> -             gcc_unreachable ();
> -           }
> -       }
> -      else
> -       {
> -         if (!equality_code)
> -           fold_overflow_warning ("assuming signed overflow does not occur
> "
> -                                  "when changing X +- C1 cmp C2 to "
> -                                  "X cmp C2 -+ C1",
> -                                  WARN_STRICT_OVERFLOW_COMPARISON);
> -         return fold_build2_loc (loc, code, type, variable, new_const);
> -       }
> -    }
> -
>    /* For comparisons of pointers we can decompose it to a compile time
>       comparison of the base objects and the offsets into the object.
>       This requires at least one operand being an ADDR_EXPR or a
>       POINTER_PLUS_EXPR to do more than the operand_equal_p test below.  */
>    if (POINTER_TYPE_P (TREE_TYPE (arg0))
>        && (TREE_CODE (arg0) == ADDR_EXPR
>           || TREE_CODE (arg1) == ADDR_EXPR
>           || TREE_CODE (arg0) == POINTER_PLUS_EXPR
>           || TREE_CODE (arg1) == POINTER_PLUS_EXPR))
>      {
> Index: gcc/fold-const.h
> ===================================================================
> --- gcc/fold-const.h    (revision 235644)
> +++ gcc/fold-const.h    (working copy)
> @@ -79,20 +79,21 @@ extern bool fold_convertible_p (const_tr
>     fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
>  extern tree fold_convert_loc (location_t, tree, tree);
>  extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree,
> tree);
>  extern tree fold_ignored_result (tree);
>  extern tree fold_abs_const (tree, tree);
>  extern tree fold_indirect_ref_1 (location_t, tree, tree);
>  extern void fold_defer_overflow_warnings (void);
>  extern void fold_undefer_overflow_warnings (bool, const gimple *, int);
>  extern void fold_undefer_and_ignore_overflow_warnings (void);
>  extern bool fold_deferring_overflow_warnings_p (void);
> +extern void fold_overflow_warning (const char*, enum
> warn_strict_overflow_code);
>  extern int operand_equal_p (const_tree, const_tree, unsigned int);
>  extern int multiple_of_p (tree, const_tree, const_tree);
>  #define omit_one_operand(T1,T2,T3)\
>     omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
>  extern tree omit_one_operand_loc (location_t, tree, tree, tree);
>  #define omit_two_operands(T1,T2,T3,T4)\
>     omit_two_operands_loc (UNKNOWN_LOCATION, T1, T2, T3, T4)
>  extern tree omit_two_operands_loc (location_t, tree, tree, tree, tree);
>  #define invert_truthvalue(T)\
>     invert_truthvalue_loc (UNKNOWN_LOCATION, T)
> Index: gcc/gimple-fold.c
> ===================================================================
> --- gcc/gimple-fold.c   (revision 235644)
> +++ gcc/gimple-fold.c   (working copy)
> @@ -3518,21 +3518,23 @@ maybe_canonicalize_mem_ref_addr (tree *t
>  }
>
>  /* Worker for both fold_stmt and fold_stmt_inplace.  The INPLACE argument
>     distinguishes both cases.  */
>
>  static bool
>  fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize)
> (tree))
>  {
>    bool changed = false;
>    gimple *stmt = gsi_stmt (*gsi);
> +  bool nowarning = gimple_no_warning_p (stmt);
>    unsigned i;
> +  fold_defer_overflow_warnings ();
>
>    /* First do required canonicalization of [TARGET_]MEM_REF addresses
>       after propagation.
>       ???  This shouldn't be done in generic folding but in the
>       propagation helpers which also know whether an address was
>       propagated.
>       Also canonicalize operand order.  */
>    switch (gimple_code (stmt))
>      {
>      case GIMPLE_ASSIGN:
> @@ -3811,20 +3813,21 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>         {
>           tree new_lhs = maybe_fold_reference (lhs, true);
>           if (new_lhs)
>             {
>               gimple_set_lhs (stmt, new_lhs);
>               changed = true;
>             }
>         }
>      }
>
> +  fold_undefer_overflow_warnings (changed && !nowarning, stmt, 0);
>    return changed;
>  }
>
>  /* Valueziation callback that ends up not following SSA edges.  */
>
>  tree
>  no_follow_ssa_edges (tree)
>  {
>    return NULL_TREE;
>  }
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd        (revision 235644)
> +++ gcc/match.pd        (working copy)
> @@ -3179,10 +3179,54 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (simplify
>   /* signbit(x) -> 0 if x is nonnegative.  */
>   (SIGNBIT tree_expr_nonnegative_p@0)
>   { integer_zero_node; })
>
>  (simplify
>   /* signbit(x) -> x<0 if x doesn't have signed zeros.  */
>   (SIGNBIT @0)
>   (if (!HONOR_SIGNED_ZEROS (@0))
>    (convert (lt @0 { build_real (TREE_TYPE (@0), dconst0); }))))
> +
> +/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
> +(for cmp (eq ne)
> + (for op (plus minus)
> +      rop (minus plus)
> +  (simplify
> +   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
> +   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
> +       && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
> +       && !TYPE_OVERFLOW_TRAPS (TREE_TYPE (@0))
> +       && !TYPE_SATURATING (TREE_TYPE (@0)))
> +    (with { tree res = int_const_binop (rop, @2, @1); }
> +     (if (TREE_OVERFLOW (res))
> +      { constant_boolean_node (cmp == NE_EXPR, type); }
> +      (if (single_use (@3))
> +       (cmp @0 { res; }))))))))
> +(for cmp (lt le gt ge)
> + (for op (plus minus)
> +      rop (minus plus)
> +  (simplify
> +   (cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
> +   (if (!TREE_OVERFLOW (@1) && !TREE_OVERFLOW (@2)
> +       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
> +    (with { tree res = int_const_binop (rop, @2, @1); }
> +     (if (TREE_OVERFLOW (res))
> +      {
> +       fold_overflow_warning (("assuming signed overflow does not occur "
> +                               "when simplifying conditional to constant"),
> +                              WARN_STRICT_OVERFLOW_CONDITIONAL);
> +        bool less = cmp == LE_EXPR || cmp == LT_EXPR;
> +       /* wi::ges_p (@2, 0) should be sufficient for a signed type.  */
> +       bool ovf_high = wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1)))
> +                       != (op == MINUS_EXPR);
> +       constant_boolean_node (less == ovf_high, type);
> +      }
> +      (if (single_use (@3))
> +       (with
> +       {
> +         fold_overflow_warning (("assuming signed overflow does not occur "
> +                                 "when changing X +- C1 cmp C2 to "
> +                                 "X cmp C2 -+ C1"),
> +                                WARN_STRICT_OVERFLOW_COMPARISON);
> +       }
> +       (cmp @0 { res; })))))))))
> Index: gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c  (revision 235644)
> +++ gcc/testsuite/gcc.dg/tree-ssa/20040305-1.c  (working copy)
> @@ -16,15 +16,15 @@ void foo(int edx, int eax)
>      }
>    if (eax == 100)
>      {
>        if (-- edx == 0)
>          afred[0] = 2;
>      }
>  }
>
>
>  /* Verify that we did a forward propagation.  */
> -/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
> +/* { dg-final { scan-tree-dump-times "gimple_simplified" 1 "forwprop1"} }
> */
>
>  /* After cddce we should have two IF statements remaining as the other
>     two tests can be threaded.  */
>  /* { dg-final { scan-tree-dump-times "if " 2 "cddce2"} } */
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-05-02  8:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-24 17:14 Move "X +- C1 CMP C2 to X CMP C2 -+ C1" to match.pd Marc Glisse
2016-04-26 11:58 ` Richard Biener
2016-04-26 20:28   ` Marc Glisse
2016-04-27  5:34     ` Marc Glisse
2016-04-27 12:02       ` Richard Biener
2016-04-28 20:15         ` Marc Glisse
2016-04-29  8:08           ` Richard Biener
2016-04-29 17:36             ` Marc Glisse
2016-05-02  8:29               ` Richard Biener
2016-04-27 11:52     ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).