diff mbox

[PR,57371] Remove useless floating point casts in comparisons

Message ID alpine.DEB.2.20.1707021922330.2564@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse July 2, 2017, 5:32 p.m. UTC
On Sun, 2 Jul 2017, Yuri Gribov wrote:

> This is initial patch for
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57371 .

Thanks. I have had this unfinished, probably wrong patch on my hard drive 
for 4 years, I doubt there is much you can extract from it, but just in 
case...

Comments

Yuri Gribov July 2, 2017, 8:23 p.m. UTC | #1
On Sun, Jul 2, 2017 at 6:32 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sun, 2 Jul 2017, Yuri Gribov wrote:
>
>> This is initial patch for
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57371 .
>
> Thanks. I have had this unfinished, probably wrong patch on my hard drive
> for 4 years, I doubt there is much you can extract from it, but just in
> case...

Thanks, this will indeed help! I only don't like !uns here:

+      bool uns = TYPE_UNSIGNED (itype);
+      bool fits = mantissa >= prec - !uns;

as I believe for INT?_MIN we need 'prec' number of bits.

-Y
Marc Glisse July 2, 2017, 8:37 p.m. UTC | #2
On Sun, 2 Jul 2017, Yuri Gribov wrote:

> On Sun, Jul 2, 2017 at 6:32 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Sun, 2 Jul 2017, Yuri Gribov wrote:
>>
>>> This is initial patch for
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57371 .
>>
>> Thanks. I have had this unfinished, probably wrong patch on my hard drive
>> for 4 years, I doubt there is much you can extract from it, but just in
>> case...
>
> Thanks, this will indeed help! I only don't like !uns here:
>
> +      bool uns = TYPE_UNSIGNED (itype);
> +      bool fits = mantissa >= prec - !uns;
>
> as I believe for INT?_MIN we need 'prec' number of bits.

INT_MIN is (minus) a power of 2, so 1 bit of mantissa is sufficient to 
represent it exactly (as long as the exponent also fits). At least for an 
IEEE754 representation using base 2, but I expect non-IEEE representations 
of floats are similar enough.
diff mbox

Patch

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 209514)
+++ gcc/fold-const.c	(working copy)
@@ -6389,21 +6389,21 @@  fold_inf_compare (location_t loc, enum t
       /* x > +Inf is always false, if with ignore sNANs.  */
       if (HONOR_SNANS (mode))
         return NULL_TREE;
       return omit_one_operand_loc (loc, type, integer_zero_node, arg0);
 
     case LE_EXPR:
       /* x <= +Inf is always true, if we don't case about NaNs.  */
       if (! HONOR_NANS (mode))
 	return omit_one_operand_loc (loc, type, integer_one_node, arg0);
 
-      /* x <= +Inf is the same as x == x, i.e. isfinite(x).  */
+      /* x <= +Inf is the same as x == x, i.e. !isnan(x).  */
       arg0 = save_expr (arg0);
       return fold_build2_loc (loc, EQ_EXPR, type, arg0, arg0);
 
     case EQ_EXPR:
     case GE_EXPR:
       /* x == +Inf and x >= +Inf are always equal to x > DBL_MAX.  */
       real_maxval (&max, neg, mode);
       return fold_build2_loc (loc, neg ? LT_EXPR : GT_EXPR, type,
 			  arg0, build_real (TREE_TYPE (arg0), max));
 
@@ -9389,70 +9389,121 @@  fold_comparison (location_t loc, enum tr
 
   tem = maybe_canonicalize_comparison (loc, code, type, arg0, arg1);
   if (tem)
     return tem;
 
   if (FLOAT_TYPE_P (TREE_TYPE (arg0)))
     {
       tree targ0 = strip_float_extensions (arg0);
       tree targ1 = strip_float_extensions (arg1);
       tree newtype = TREE_TYPE (targ0);
+      tree ftype = TREE_TYPE (arg0);
 
       if (TYPE_PRECISION (TREE_TYPE (targ1)) > TYPE_PRECISION (newtype))
 	newtype = TREE_TYPE (targ1);
 
       /* Fold (double)float1 CMP (double)float2 into float1 CMP float2.  */
-      if (TYPE_PRECISION (newtype) < TYPE_PRECISION (TREE_TYPE (arg0)))
+      if (TYPE_PRECISION (newtype) < TYPE_PRECISION (ftype))
 	return fold_build2_loc (loc, code, type,
 			    fold_convert_loc (loc, newtype, targ0),
 			    fold_convert_loc (loc, newtype, targ1));
 
       /* (-a) CMP (-b) -> b CMP a  */
       if (TREE_CODE (arg0) == NEGATE_EXPR
 	  && TREE_CODE (arg1) == NEGATE_EXPR)
 	return fold_build2_loc (loc, code, type, TREE_OPERAND (arg1, 0),
 			    TREE_OPERAND (arg0, 0));
 
       if (TREE_CODE (arg1) == REAL_CST)
 	{
 	  REAL_VALUE_TYPE cst;
 	  cst = TREE_REAL_CST (arg1);
 
 	  /* (-a) CMP CST -> a swap(CMP) (-CST)  */
 	  if (TREE_CODE (arg0) == NEGATE_EXPR)
 	    return fold_build2_loc (loc, swap_tree_comparison (code), type,
 				TREE_OPERAND (arg0, 0),
-				build_real (TREE_TYPE (arg1),
+				build_real (ftype,
 					    real_value_negate (&cst)));
 
 	  /* IEEE doesn't distinguish +0 and -0 in comparisons.  */
 	  /* a CMP (-0) -> a CMP 0  */
 	  if (REAL_VALUE_MINUS_ZERO (cst))
 	    return fold_build2_loc (loc, code, type, arg0,
-				build_real (TREE_TYPE (arg1), dconst0));
+				build_real (ftype, dconst0));
 
 	  /* x != NaN is always true, other ops are always false.  */
 	  if (REAL_VALUE_ISNAN (cst)
-	      && ! HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1))))
+	      && ! HONOR_SNANS (TYPE_MODE (ftype)))
 	    {
 	      tem = (code == NE_EXPR) ? integer_one_node : integer_zero_node;
 	      return omit_one_operand_loc (loc, type, tem, arg0);
 	    }
 
 	  /* Fold comparisons against infinity.  */
 	  if (REAL_VALUE_ISINF (cst)
-	      && MODE_HAS_INFINITIES (TYPE_MODE (TREE_TYPE (arg1))))
+	      && MODE_HAS_INFINITIES (TYPE_MODE (ftype)))
 	    {
 	      tem = fold_inf_compare (loc, code, type, arg0, arg1);
 	      if (tem != NULL_TREE)
 		return tem;
 	    }
+
+	  /* (double)i CMP cst is often just i CMP cst'.
+	     See PR 57371 for a detailed analysis and other ideas.  */
+	  if (TREE_CODE (arg0) == FLOAT_EXPR)
+	    {
+	      tree inner = TREE_OPERAND (arg0, 0);
+	      tree itype = TREE_TYPE (inner);
+	      unsigned mantissa = significand_size (TYPE_MODE (ftype));
+	      unsigned prec = element_precision (itype);
+	      bool uns = TYPE_UNSIGNED (itype);
+	      bool fits = mantissa >= prec - !uns;
+	      /* If ftype cannot represent exactly all values of itype,
+		 we may have an inexact exception.  If the conversion from
+		 itype to ftype may overflow (unsigned __int128 to float),
+		 we may have an overflow exception.  */
+	      if (!flag_trapping_math || fits)
+		{
+		  /* Comparison with 0 is always fine.  */
+		  if (real_zerop (arg1))
+		    return fold_build2_loc (loc, code, type, inner,
+					    build_zero_cst (itype));
+
+		  /* (double)i == 2.5 is false.  This may assume that HUGE_VAL
+		     is an integer.  */
+		  if (!real_isinteger (&cst, TYPE_MODE (ftype)))
+		    {
+		      if (code == EQ_EXPR)
+			return omit_one_operand_loc (loc, type,
+						     integer_zero_node, arg0);
+		      if (code == NE_EXPR)
+			return omit_one_operand_loc (loc, type,
+						     integer_one_node, arg0);
+		    }
+
+		  /* floor(abs(cst))+1 is representable exactly in ftype.  */
+		  int log_cst = real_exponent (&cst);
+		  gcc_assert (REAL_MODE_FORMAT (TYPE_MODE (ftype))->b % 2 == 0);
+		  if (log_cst <= mantissa)
+		    {
+		      tree imin = TYPE_MIN_VALUE (itype);
+		      tree imax = TYPE_MAX_VALUE (itype);
+		      REAL_VALUE_TYPE iminf = real_value_from_int_cst (imin);
+		      REAL_VALUE_TYPE imaxf = real_value_from_int_cst (imax);
+		      if (REAL_VALUES_LESS (cst, iminf)
+			  || REAL_VALUES_LESS (imaxf, cst))
+			return build_int_cst (type,
+				 real_compare (code, &dconst0, &cst));
+		    }
+		}
+	    }
 	}
 
       /* If this is a comparison of a real constant with a PLUS_EXPR
 	 or a MINUS_EXPR of a real constant, we can convert it into a
 	 comparison with a revised real constant as long as no overflow
 	 occurs when unsafe_math_optimizations are enabled.  */
       if (flag_unsafe_math_optimizations
 	  && TREE_CODE (arg1) == REAL_CST
 	  && (TREE_CODE (arg0) == PLUS_EXPR
 	      || TREE_CODE (arg0) == MINUS_EXPR)
Index: gcc/testsuite/gcc.dg/pr57371-1.c
===================================================================
--- gcc/testsuite/gcc.dg/pr57371-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr57371-1.c	(working copy)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized -ftrapping-math" } */
+
+int f (char x)
+{
+  long double y = x;
+  return y <= 0;
+}
+
+/* { dg-final { scan-tree-dump-not "double" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: gcc/testsuite/gcc.dg/pr57371-1.c
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: gcc/testsuite/gcc.dg/pr57371-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pr57371-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr57371-2.c	(working copy)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized -fno-trapping-math" } */
+
+int f (unsigned long long x)
+{
+  float y = x;
+  return y <= 0;
+}
+
+/* { dg-final { scan-tree-dump-not "float" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: gcc/testsuite/gcc.dg/pr57371-2.c
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: gcc/testsuite/gcc.dg/pr57371-3.c
===================================================================
--- gcc/testsuite/gcc.dg/pr57371-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr57371-3.c	(working copy)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized -ftrapping-math" } */
+
+int f (unsigned long long x)
+{
+  float y = x;
+  return y <= 0;
+}
+
+/* { dg-final { scan-tree-dump "float" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */