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 2/9][GCC][AArch64][middle-end] Add rules to strip away unneeded type casts in expressions


Hi Joseph,

This is an updated version of my first patch which moves part of the type conversion code
from convert.c to match.pd because match.pd is able to apply this transformation in the
presence of intermediate temporary variables.

I couldn't move the error parts as match.pd doesn't seem to have access to the diagnostics
bits and also I feel like this may be a bit against the philosophy of match.pd. 

Regtested and bootstrapped on aarch64-none-linux-gnu and no issues.

Ok for trunk?

Thanks,
Tamar

Concretely it makes both these cases behave the same

  float e = (float)a * (float)b;
  *c = (_Float16)e;

and 

  *c = (_Float16)((float)a * (float)b);

Thanks,
Tamar

gcc/ChangeLog:

2018-11-28  Tamar Christina  <tamar.christina@arm.com>

	* convert.c (convert_to_real_1): Move part of conversion code...
	* match.pd: ...To here.

gcc/testsuite/ChangeLog:

2018-11-28  Tamar Christina  <tamar.christina@arm.com>

	* gcc.dg/type-convert-var.c: New test.

> -----Original Message-----
> From: Joseph Myers <joseph@codesourcery.com>
> Sent: Tuesday, November 13, 2018 17:59
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; James Greenhalgh
> <James.Greenhalgh@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>; law@redhat.com; ian@airs.com;
> rguenther@suse.de
> Subject: RE: [PATCH 2/9][GCC][AArch64][middle-end] Add rules to strip away
> unneeded type casts in expressions
> 
> On Tue, 13 Nov 2018, Tamar Christina wrote:
> 
> > Would restricting it to flag_unsafe_math_optimizations not be enough
> > in this case? Since if it's only done for unsafe math then you likely
> > won't care about a small loss in precision anyway?
> 
> We have what should be the right logic (modulo DFP issues) in
> convert_to_real_1, for converting (outertype)((innertype0)a+(innertype1)b)
> into ((newtype)a+(newtype)b).  I think the right thing to do is to move that
> logic, including the associated comments, from convert_to_real_1 to
> match.pd (not duplicate it or a subset in both places - make match.pd able to
> do everything that logic in convert.c does, so it's no longer needed in
> convert.c).  That logic knows both when the conversion is OK based on
> flag_unsafe_math_optimizations, and when it's OK based on
> real_can_shorten_arithmetic.
> 
> (Most of the other special case logic in convert_to_real_1 to optimize
> particular conversions would make sense to move as well - but for your
> present issue, only the PLUS_EXPR / MINUS_EXPR / MULT_EXPR /
> RDIV_EXPR logic should need to move.)
> 
> > But my simple testcase is
> 
> I'd hope a simpler test could be added - one not involving complex arithmetic
> at all, just an intermediate temporary variable or whatever's needed for
> convert_to_real_1 not to apply.
> 
> --
> Joseph S. Myers
> joseph@codesourcery.com
diff --git a/gcc/convert.c b/gcc/convert.c
index 68705f3e9b09c5d0e6bae0ba1c7bca0db7107980..73ca22967d83aa125ad0bd67e8070a775eaf8d14 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -272,92 +272,6 @@ convert_to_real_1 (tree type, tree expr, bool fold_p)
 	      return build1 (TREE_CODE (expr), type, arg);
 	    }
 	  break;
-	/* Convert (outertype)((innertype0)a+(innertype1)b)
-	   into ((newtype)a+(newtype)b) where newtype
-	   is the widest mode from all of these.  */
-	case PLUS_EXPR:
-	case MINUS_EXPR:
-	case MULT_EXPR:
-	case RDIV_EXPR:
-	   {
-	     tree arg0 = strip_float_extensions (TREE_OPERAND (expr, 0));
-	     tree arg1 = strip_float_extensions (TREE_OPERAND (expr, 1));
-
-	     if (FLOAT_TYPE_P (TREE_TYPE (arg0))
-		 && FLOAT_TYPE_P (TREE_TYPE (arg1))
-		 && DECIMAL_FLOAT_TYPE_P (itype) == DECIMAL_FLOAT_TYPE_P (type))
-	       {
-		  tree newtype = type;
-
-		  if (TYPE_MODE (TREE_TYPE (arg0)) == SDmode
-		      || TYPE_MODE (TREE_TYPE (arg1)) == SDmode
-		      || TYPE_MODE (type) == SDmode)
-		    newtype = dfloat32_type_node;
-		  if (TYPE_MODE (TREE_TYPE (arg0)) == DDmode
-		      || TYPE_MODE (TREE_TYPE (arg1)) == DDmode
-		      || TYPE_MODE (type) == DDmode)
-		    newtype = dfloat64_type_node;
-		  if (TYPE_MODE (TREE_TYPE (arg0)) == TDmode
-		      || TYPE_MODE (TREE_TYPE (arg1)) == TDmode
-		      || TYPE_MODE (type) == TDmode)
-                    newtype = dfloat128_type_node;
-		  if (newtype == dfloat32_type_node
-		      || newtype == dfloat64_type_node
-		      || newtype == dfloat128_type_node)
-		    {
-		      expr = build2 (TREE_CODE (expr), newtype,
-				     convert_to_real_1 (newtype, arg0,
-							fold_p),
-				     convert_to_real_1 (newtype, arg1,
-							fold_p));
-		      if (newtype == type)
-			return expr;
-		      break;
-		    }
-
-		  if (TYPE_PRECISION (TREE_TYPE (arg0)) > TYPE_PRECISION (newtype))
-		    newtype = TREE_TYPE (arg0);
-		  if (TYPE_PRECISION (TREE_TYPE (arg1)) > TYPE_PRECISION (newtype))
-		    newtype = TREE_TYPE (arg1);
-		  /* Sometimes this transformation is safe (cannot
-		     change results through affecting double rounding
-		     cases) and sometimes it is not.  If NEWTYPE is
-		     wider than TYPE, e.g. (float)((long double)double
-		     + (long double)double) converted to
-		     (float)(double + double), the transformation is
-		     unsafe regardless of the details of the types
-		     involved; double rounding can arise if the result
-		     of NEWTYPE arithmetic is a NEWTYPE value half way
-		     between two representable TYPE values but the
-		     exact value is sufficiently different (in the
-		     right direction) for this difference to be
-		     visible in ITYPE arithmetic.  If NEWTYPE is the
-		     same as TYPE, however, the transformation may be
-		     safe depending on the types involved: it is safe
-		     if the ITYPE has strictly more than twice as many
-		     mantissa bits as TYPE, can represent infinities
-		     and NaNs if the TYPE can, and has sufficient
-		     exponent range for the product or ratio of two
-		     values representable in the TYPE to be within the
-		     range of normal values of ITYPE.  */
-		  if (TYPE_PRECISION (newtype) < TYPE_PRECISION (itype)
-		      && (flag_unsafe_math_optimizations
-			  || (TYPE_PRECISION (newtype) == TYPE_PRECISION (type)
-			      && real_can_shorten_arithmetic (TYPE_MODE (itype),
-							      TYPE_MODE (type))
-			      && !excess_precision_type (newtype))))
-		    {
-		      expr = build2 (TREE_CODE (expr), newtype,
-				     convert_to_real_1 (newtype, arg0,
-							fold_p),
-				     convert_to_real_1 (newtype, arg1,
-							fold_p));
-		      if (newtype == type)
-			return expr;
-		    }
-	       }
-	   }
-	  break;
 	default:
 	  break;
       }
diff --git a/gcc/match.pd b/gcc/match.pd
index d07ceb7d087b8b5c5a7d7362ad9d8f71ac90dc08..5f7b20a164be11a9e2b2b8449521cb29868ca9f9 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4709,6 +4709,83 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 (convert (op (convert:utype @0)
 		      (convert:utype @1))))))))
 
+/* Convert (outertype)((innertype0)a+(innertype1)b)
+   into ((newtype)a+(newtype)b) where newtype
+   is the widest mode from all of these.  */
+(for op (plus minus mult rdiv)
+ (simplify
+   (convert (op:s@0 (convert1? @1) (convert2? @2)))
+   (with { tree arg0 = strip_float_extensions (@1);
+	   tree arg1 = strip_float_extensions (@2);
+	   tree itype = TREE_TYPE (@0);
+	   tree ty1 = TREE_TYPE (arg0);
+	   tree ty2 = TREE_TYPE (arg1);
+	   enum tree_code code = TREE_CODE (itype); }
+    (if (FLOAT_TYPE_P (ty1)
+	 && FLOAT_TYPE_P (ty2)
+	 && FLOAT_TYPE_P (type)
+	 && DECIMAL_FLOAT_TYPE_P (itype) == DECIMAL_FLOAT_TYPE_P (type))
+       (with { tree newtype = type;
+	       if (TYPE_MODE (ty1) == SDmode
+		   || TYPE_MODE (ty2) == SDmode
+		   || TYPE_MODE (type) == SDmode)
+		 newtype = dfloat32_type_node;
+	       if (TYPE_MODE (ty1) == DDmode
+		   || TYPE_MODE (ty2) == DDmode
+		   || TYPE_MODE (type) == DDmode)
+		 newtype = dfloat64_type_node;
+	       if (TYPE_MODE (ty1) == TDmode
+		   || TYPE_MODE (ty2) == TDmode
+		   || TYPE_MODE (type) == TDmode)
+		 newtype = dfloat128_type_node; }
+	(if ((newtype == dfloat32_type_node
+	      || newtype == dfloat64_type_node
+	      || newtype == dfloat128_type_node)
+	     && newtype == type)
+	   (convert:newtype (op (convert:newtype @1) (convert:newtype @2)))
+	   (with { if (TYPE_PRECISION (ty1) > TYPE_PRECISION (newtype))
+		     newtype = ty1;
+		   if (TYPE_PRECISION (ty2) > TYPE_PRECISION (newtype))
+		     newtype = ty2; }
+	     /* Sometimes this transformation is safe (cannot
+	        change results through affecting double rounding
+	        cases) and sometimes it is not.  If NEWTYPE is
+	        wider than TYPE, e.g. (float)((long double)double
+	        + (long double)double) converted to
+	        (float)(double + double), the transformation is
+	        unsafe regardless of the details of the types
+	        involved; double rounding can arise if the result
+	        of NEWTYPE arithmetic is a NEWTYPE value half way
+	        between two representable TYPE values but the
+	        exact value is sufficiently different (in the
+	        right direction) for this difference to be
+	        visible in ITYPE arithmetic.  If NEWTYPE is the
+	        same as TYPE, however, the transformation may be
+	        safe depending on the types involved: it is safe
+	        if the ITYPE has strictly more than twice as many
+	        mantissa bits as TYPE, can represent infinities
+	        and NaNs if the TYPE can, and has sufficient
+	        exponent range for the product or ratio of two
+	        values representable in the TYPE to be within the
+	        range of normal values of ITYPE.  */
+	     (if (TYPE_PRECISION (newtype) < TYPE_PRECISION (itype)
+		  && (flag_unsafe_math_optimizations
+		      || (TYPE_PRECISION (newtype) == TYPE_PRECISION (type)
+			  && real_can_shorten_arithmetic (TYPE_MODE (itype),
+							  TYPE_MODE (type))
+			  && !excess_precision_type (newtype))))
+		(convert:newtype (op (convert:newtype @1) (convert:newtype @2)))
+	))))
+	(if (code == REAL_TYPE)
+	  /* Ignore the conversion if we don't need to store intermediate
+	     results and neither type is a decimal float.  */
+	  (if (flag_float_store
+	       || DECIMAL_FLOAT_TYPE_P (type)
+	       || DECIMAL_FLOAT_TYPE_P (itype))
+	    (convert:type (op (convert:ty1 @1) (convert:ty2 @2)))
+	    (nop:type (op (convert:ty1 @1) (convert:ty2 @2))))))
+)))
+
 /* This is another case of narrowing, specifically when there's an outer
    BIT_AND_EXPR which masks off bits outside the type of the innermost
    operands.   Like the previous case we have to convert the operands
diff --git a/gcc/testsuite/gcc.dg/type-convert-var.c b/gcc/testsuite/gcc.dg/type-convert-var.c
new file mode 100644
index 0000000000000000000000000000000000000000..88d74e2a49d7123515b87ff64a18bd9b306d57e9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/type-convert-var.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O1 -fdump-tree-optimized" } */
+void foo (float a, float b, float *c)
+{
+  double e = (double)a * (double)b;
+  *c = (float)e;
+}
+
+/* { dg-final { scan-tree-dump-not {double} "optimized" } } */


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