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

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

Re: [PATCH] TYPE_OVERFLOW_* cleanup

From: Marc Glisse <marc dot glisse at inria dot fr>
To: Marek Polacek <polacek at redhat dot com>
Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>
Date: Tue, 9 Dec 2014 20:26:56 +0100 (CET)
Subject: Re: [PATCH] TYPE_OVERFLOW_* cleanup
Authentication-results: sourceware.org; auth=none
References: <20141209182958 dot GQ16332 at redhat dot com>
Reply-to: gcc-patches at gcc dot gnu dot org

On Tue, 9 Dec 2014, Marek Polacek wrote:

The issue here is that TYPE_OVERFLOW_TRAPS, TYPE_OVERFLOW_UNDEFINED,
and TYPE_OVERFLOW_WRAPS macros work on integral types only, yet we
pass e.g. pointer_type/real_type to them.  This patch adds proper
checking for these macros and adds missing guards to various places.
This looks pretty straightforward, but I had to tweak a few places
wrt vector_types (where I've used VECTOR_INTEGER_TYPE_P) to not to
regress folding - and I'm afraid I missed places that aren't tested
in our testsuite :/.

Bootstrapped/regtested on ppc64-linux and x86_64-linux.

2014-12-09  Marek Polacek  <polacek@redhat.com>

	* fold-const.c (negate_expr_p): Check for INTEGRAL_TYPE_P.
	(fold_negate_expr): Likewise.
	(extract_muldiv_1): Likewise.
	(maybe_canonicalize_comparison_1): Likewise.
	(fold_comparison): Likewise.
	(fold_binary_loc): Likewise.
	(tree_binary_nonnegative_warnv_p): Likewise.
	(tree_binary_nonzero_warnv_p): Likewise.
	* gimple-ssa-strength-reduction.c (legal_cast_p_1): Likewise.
	* tree-scalar-evolution.c (simple_iv): Likewise.
	(scev_const_prop): Likewise.
	* tree-ssa-loop-niter.c (expand_simple_operations): Likewise.
	* match.pd (X % -C): Likewise.
	(-A - 1 -> ~A): Likewise.
	(~A + A -> -1): Check for INTEGRAL_TYPE_P or VECTOR_INTEGER_TYPE_P.
	* tree-vect-generic.c (expand_vector_operation): Likewise.
	* tree.h (INTEGRAL_TYPE_CHECK): Define.
	(TYPE_OVERFLOW_WRAPS, TYPE_OVERFLOW_UNDEFINED, TYPE_OVERFLOW_TRAPS):
	Add INTEGRAL_TYPE_CHECK.

diff --git gcc/fold-const.c gcc/fold-const.c
index 0c4fe40..ff9d917 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -426,7 +426,8 @@ negate_expr_p (tree t)

    case VECTOR_CST:
      {
-	if (FLOAT_TYPE_P (TREE_TYPE (type)) || TYPE_OVERFLOW_WRAPS (type))
+	if (FLOAT_TYPE_P (TREE_TYPE (type))
+	    || (INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_WRAPS (type)))
	  return true;


type is a vector type, so INTEGRAL_TYPE_P (type) is always false I think.


	int count = TYPE_VECTOR_SUBPARTS (type), i;
@@ -558,7 +559,8 @@ fold_negate_expr (location_t loc, tree t)
    case INTEGER_CST:
      tem = fold_negate_const (t, type);
      if (TREE_OVERFLOW (tem) == TREE_OVERFLOW (t)
-	  || (!TYPE_OVERFLOW_TRAPS (type)
+	  || (INTEGRAL_TYPE_P (type)


Can that be false for an INTEGER_CST?

+	      && !TYPE_OVERFLOW_TRAPS (type)
	      && TYPE_OVERFLOW_WRAPS (type))
	  || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
	return tem;
@@ -5951,7 +5953,8 @@ extract_muldiv_1 (tree t, tree c, enum tree_code code, tree wide_type,
	   || EXPRESSION_CLASS_P (op0))
	  /* ... and has wrapping overflow, and its type is smaller
	     than ctype, then we cannot pass through as widening.  */
-	  && ((TYPE_OVERFLOW_WRAPS (TREE_TYPE (op0))
+	  && (((INTEGRAL_TYPE_P (TREE_TYPE (op0))
+		&& TYPE_OVERFLOW_WRAPS (TREE_TYPE (op0)))
	       && (TYPE_PRECISION (ctype)
	           > TYPE_PRECISION (TREE_TYPE (op0))))
	      /* ... or this is a truncation (t is narrower than op0),
@@ -5966,7 +5969,8 @@ extract_muldiv_1 (tree t, tree c, enum tree_code code, tree wide_type,
	      /* ... or has undefined overflow while the converted to
		 type has not, we cannot do the operation in the inner type
		 as that would introduce undefined overflow.  */
-	      || (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (op0))
+	      || ((INTEGRAL_TYPE_P (TREE_TYPE (op0))
+		   && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (op0)))
		  && !TYPE_OVERFLOW_UNDEFINED (type))))
	break;

@@ -6159,6 +6163,7 @@ extract_muldiv_1 (tree t, tree c, enum tree_code code, tree wide_type,
      if ((code == TRUNC_MOD_EXPR || code == CEIL_MOD_EXPR
	   || code == FLOOR_MOD_EXPR || code == ROUND_MOD_EXPR)
	  /* If the multiplication can overflow we cannot optimize this.  */
+	  && INTEGRAL_TYPE_P (TREE_TYPE (t))
	  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (t))
	  && TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST
	  && wi::multiple_of_p (op1, c, TYPE_SIGN (type)))
@@ -6211,7 +6216,8 @@ extract_muldiv_1 (tree t, tree c, enum tree_code code, tree wide_type,

	 If we have an unsigned type, we cannot do this since it will change
	 the result if the original computation overflowed.  */
-      if (TYPE_OVERFLOW_UNDEFINED (ctype)
+      if (INTEGRAL_TYPE_P (ctype)
+	  && TYPE_OVERFLOW_UNDEFINED (ctype)
	  && ((code == MULT_EXPR && tcode == EXACT_DIV_EXPR)
	      || (tcode == MULT_EXPR
		  && code != TRUNC_MOD_EXPR && code != CEIL_MOD_EXPR
@@ -8497,7 +8503,8 @@ maybe_canonicalize_comparison_1 (location_t loc, enum tree_code code, tree type,

  /* Match A +- CST code arg1 and CST code arg1.  We can change the
     first form only if overflow is undefined.  */
-  if (!((TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))
+  if (!(((INTEGRAL_TYPE_P (TREE_TYPE (arg0))
+	  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
	 /* In principle pointers also have undefined overflow behavior,
	    but that causes problems elsewhere.  */
	 && !POINTER_TYPE_P (TREE_TYPE (arg0))
@@ -8712,7 +8719,9 @@ fold_comparison (location_t loc, enum tree_code code, tree type,

  /* 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 || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
+      && (equality_code
+	  || (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
@@ -9031,7 +9040,8 @@ fold_comparison (location_t loc, enum tree_code code, tree type,
     X CMP Y +- C2 +- C1 for signed X, Y.  This is valid if
     the resulting offset is smaller in absolute value than the
     original one and has the same sign.  */
-  if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))
+  if ((INTEGRAL_TYPE_P (TREE_TYPE (arg0))
+       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
      && (TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
      && (TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
	  && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1)))
@@ -9085,7 +9095,8 @@ fold_comparison (location_t loc, enum tree_code code, tree type,
     signed arithmetic case.  That form is created by the compiler
     often enough for folding it to be of value.  One example is in
     computing loop trip counts after Operator Strength Reduction.  */
-  if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))
+  if ((INTEGRAL_TYPE_P (TREE_TYPE (arg0))
+       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
      && TREE_CODE (arg0) == MULT_EXPR
      && (TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
          && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1)))
@@ -10074,7 +10085,8 @@ fold_binary_loc (location_t loc,
	  /* Reassociate (plus (plus (mult) (foo)) (mult)) as
	     (plus (plus (mult) (mult)) (foo)) so that we can
	     take advantage of the factoring cases below.  */
-	  if (TYPE_OVERFLOW_WRAPS (type)
+	  if (INTEGRAL_TYPE_P (type)
+	      && TYPE_OVERFLOW_WRAPS (type)
	      && (((TREE_CODE (arg0) == PLUS_EXPR
		    || TREE_CODE (arg0) == MINUS_EXPR)
		   && TREE_CODE (arg1) == MULT_EXPR)

Is there a particular reason to disable this for vectors? Note that we arealready in a !FLOAT_TYPE_P block.

@@ -14725,7 +14737,8 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0,
      break;

    case MULT_EXPR:
-      if (FLOAT_TYPE_P (type) || TYPE_OVERFLOW_UNDEFINED (type))
+      if (FLOAT_TYPE_P (type) || (INTEGRAL_TYPE_P (type)
+				  && TYPE_OVERFLOW_UNDEFINED (type)))
	{
	  /* x * x is always non-negative for floating point x
	     or without overflow.  */


Again, why disable this for vectors?
(Btw tree_unary_nonnegative_warnv_p seems wrong for ABS of an integer vector)

I am stopping here. I think by default you should allow integer vectors.
A new macro that accepts both scalar and vector integer types would be
helpful (like FLOAT_TYPE_P).

--
Marc Glisse

Follow-Ups:
- Re: [PATCH] TYPE_OVERFLOW_* cleanup
  - From: Marek Polacek

References:
- [PATCH] TYPE_OVERFLOW_* cleanup
  - From: Marek Polacek

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