[GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

Hi All,

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.

The previous patch was only regtested on aarch64-none-linux-gnu and I hadn't done a
regression on x86_64-pc-linux-gnu only a bootstrap.  The previous patch was approved

here https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00116.html
but before committing I ran a x86_64-pc-linux-gnu regtest to be sure and this
showed an issue with a DFP test. I Have fixed this by removing the offending convert.
The convert was just saying "keep the type as is" but match.pd looped here as it thinks
the match did something and would try other patterns, causing it to match itself again.

Instead when there's nothing to update, I just don't do anything.

The second change was to merge this with the existing pattern for integer conversion
in order to silence a warning from match.pd which though that the two patterns overlaps
because their match conditions are similar (they have different conditions inside the ifs
but match.pd doesn't check those of course.).

Regtested and bootstrapped on aarch64-none-linux-gnu and x86_64-pc-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:

2019-01-04  Tamar Christina  <tamar.christina@arm.com>

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

gcc/testsuite/ChangeLog:

2019-01-04  Tamar Christina  <tamar.christina@arm.com>

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

-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb10277.patch --]
[-- Type: text/x-diff; name="rb10277.patch", Size: 11484 bytes --]

diff --git a/gcc/convert.c b/gcc/convert.c
index 1a3353c870768a33fe22480ec97c7d3e0c504075..a16b7af0ec54693eb4f1e3a110aabc1aa18eb8df 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -295,92 +295,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 97a94cd8b2f2e0fee9ffbc76c5277c97689b6f42..4e56121e75fd226fc733dfc5c239d00b690c3d03 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4758,37 +4758,115 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
    term we want to move all that code out of the front-ends into here.  */
 
-/* If we have a narrowing conversion of an arithmetic operation where
-   both operands are widening conversions from the same type as the outer
-   narrowing conversion.  Then convert the innermost operands to a suitable
-   unsigned type (to avoid introducing undefined behavior), perform the
-   operation and convert the result to the desired type.  */
-(for op (plus minus)
-  (simplify
-    (convert (op:s (convert@2 @0) (convert?@3 @1)))
-    (if (INTEGRAL_TYPE_P (type)
-	 /* We check for type compatibility between @0 and @1 below,
-	    so there's no need to check that @1/@3 are integral types.  */
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@2))
-	 /* The precision of the type of each operand must match the
-	    precision of the mode of each operand, similarly for the
-	    result.  */
-	 && type_has_mode_precision_p (TREE_TYPE (@0))
-	 && type_has_mode_precision_p (TREE_TYPE (@1))
-	 && type_has_mode_precision_p (type)
-	 /* The inner conversion must be a widening conversion.  */
-	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
-	 && types_match (@0, type)
-	 && (types_match (@0, @1)
-	     /* Or the second operand is const integer or converted const
-		integer from valueize.  */
-	     || TREE_CODE (@1) == INTEGER_CST))
-      (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
-	(op @0 (convert @1))
-	(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-	 (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?@3 @1) (convert2?@4 @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); }
+    (switch
+      (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)))
+	    (nop:type (op (convert:ty1 @1) (convert:ty2 @2)))))
+
+      /* If we have a narrowing conversion of an arithmetic operation where
+	 both operands are widening conversions from the same type as the outer
+	 narrowing conversion.  Then convert the innermost operands to a
+	 suitable unsigned type (to avoid introducing undefined behavior),
+	 perform the operation and convert the result to the desired type.  */
+      (if (INTEGRAL_TYPE_P (type)
+	   && op != MULT_EXPR
+	   && op != RDIV_EXPR
+	   /* We check for type compatibility between @0 and @1 below,
+	      so there's no need to check that @2/@4 are integral types.  */
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@3))
+	   /* The precision of the type of each operand must match the
+	      precision of the mode of each operand, similarly for the
+	      result.  */
+	   && type_has_mode_precision_p (TREE_TYPE (@1))
+	   && type_has_mode_precision_p (TREE_TYPE (@2))
+	   && type_has_mode_precision_p (type)
+	   /* The inner conversion must be a widening conversion.  */
+	   && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (TREE_TYPE (@1))
+	   && types_match (@1, type)
+	   && (types_match (@1, @2)
+	       /* Or the second operand is const integer or converted const
+		  integer from valueize.  */
+	       || TREE_CODE (@2) == INTEGER_CST))
+        (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
+	  (op @1 (convert @2))
+	  (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
+	   (convert (op (convert:utype @1)
+		        (convert:utype @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
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" } } */

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Marc Glisse]

> +	     (convert:newtype (op (convert:newtype @1) (convert:newtype @2)))

The outer 'convert' is unnecessary, op already has the same type.

> +	    (nop:type (op (convert:ty1 @1) (convert:ty2 @2)))))

Please don't use 'nop' directly, use 'convert' instead. This line is very 
suspicious, both arguments of op should have the same type. Specifying the 
outertype should be unnecessary, it is always 'type'. And if necessary, I 
expect '(convert:ty1 @1)' is the same as '{ arg0; }'.

The explicit list of types is quite ugly, but since it already exists...

-- 
Marc Glisse

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

Hi Marc

The 01/04/2019 17:50, Marc Glisse wrote:
> > +	     (convert:newtype (op (convert:newtype @1) (convert:newtype @2)))
> 
> The outer 'convert' is unnecessary, op already has the same type.
> 

Does it? The only comparison that has been done between the type of op and "type" is that
they are both a decimal floating point type. I don't see any reason why they have to be the
same type.

> > +	    (nop:type (op (convert:ty1 @1) (convert:ty2 @2)))))
> 
> Please don't use 'nop' directly, use 'convert' instead. This line is very 
> suspicious, both arguments of op should have the same type. Specifying the 
> outertype should be unnecessary, it is always 'type'. And if necessary, I 
> expect '(convert:ty1 @1)' is the same as '{ arg0; }'.
> 

Ah I wasn't aware I could use arg0 here. I've updated the patch, though I don't
really find this clearer.

Bootstrapped and regtested again on aarch64-none-linux-gnu and x86_64-pc-linux-gnu and no issues.

Ok for trunk?

Thanks,
Tamar

> The explicit list of types is quite ugly, but since it already exists...
> 
> -- 
> Marc Glisse

-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb10277.patch --]
[-- Type: text/x-diff; name="rb10277.patch", Size: 11497 bytes --]

diff --git a/gcc/convert.c b/gcc/convert.c
index 1a3353c870768a33fe22480ec97c7d3e0c504075..a16b7af0ec54693eb4f1e3a110aabc1aa18eb8df 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -295,92 +295,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 97a94cd8b2f2e0fee9ffbc76c5277c97689b6f42..ef4d9dcbc10d48b893ae5b7df06127a6dc51856a 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4758,37 +4758,115 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
    term we want to move all that code out of the front-ends into here.  */
 
-/* If we have a narrowing conversion of an arithmetic operation where
-   both operands are widening conversions from the same type as the outer
-   narrowing conversion.  Then convert the innermost operands to a suitable
-   unsigned type (to avoid introducing undefined behavior), perform the
-   operation and convert the result to the desired type.  */
-(for op (plus minus)
-  (simplify
-    (convert (op:s (convert@2 @0) (convert?@3 @1)))
-    (if (INTEGRAL_TYPE_P (type)
-	 /* We check for type compatibility between @0 and @1 below,
-	    so there's no need to check that @1/@3 are integral types.  */
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@2))
-	 /* The precision of the type of each operand must match the
-	    precision of the mode of each operand, similarly for the
-	    result.  */
-	 && type_has_mode_precision_p (TREE_TYPE (@0))
-	 && type_has_mode_precision_p (TREE_TYPE (@1))
-	 && type_has_mode_precision_p (type)
-	 /* The inner conversion must be a widening conversion.  */
-	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
-	 && types_match (@0, type)
-	 && (types_match (@0, @1)
-	     /* Or the second operand is const integer or converted const
-		integer from valueize.  */
-	     || TREE_CODE (@1) == INTEGER_CST))
-      (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
-	(op @0 (convert @1))
-	(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-	 (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?@3 @1) (convert2?@4 @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); }
+    (switch
+      (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 (op (convert:ty1 { arg0; }) (convert:ty2 { arg1; })))))
+
+      /* If we have a narrowing conversion of an arithmetic operation where
+	 both operands are widening conversions from the same type as the outer
+	 narrowing conversion.  Then convert the innermost operands to a
+	 suitable unsigned type (to avoid introducing undefined behavior),
+	 perform the operation and convert the result to the desired type.  */
+      (if (INTEGRAL_TYPE_P (type)
+	   && op != MULT_EXPR
+	   && op != RDIV_EXPR
+	   /* We check for type compatibility between @0 and @1 below,
+	      so there's no need to check that @2/@4 are integral types.  */
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@3))
+	   /* The precision of the type of each operand must match the
+	      precision of the mode of each operand, similarly for the
+	      result.  */
+	   && type_has_mode_precision_p (TREE_TYPE (@1))
+	   && type_has_mode_precision_p (TREE_TYPE (@2))
+	   && type_has_mode_precision_p (type)
+	   /* The inner conversion must be a widening conversion.  */
+	   && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (TREE_TYPE (@1))
+	   && types_match (@1, type)
+	   && (types_match (@1, @2)
+	       /* Or the second operand is const integer or converted const
+		  integer from valueize.  */
+	       || TREE_CODE (@2) == INTEGER_CST))
+        (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
+	  (op @1 (convert @2))
+	  (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
+	   (convert (op (convert:utype @1)
+		        (convert:utype @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
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" } } */

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Marc Glisse]

On Mon, 7 Jan 2019, Tamar Christina wrote:

> The 01/04/2019 17:50, Marc Glisse wrote:
>>> +	     (convert:newtype (op (convert:newtype @1) (convert:newtype @2)))
>>
>> The outer 'convert' is unnecessary, op already has the same type.
>>
>
> Does it? The only comparison that has been done between the type of op and "type" is that
> they are both a decimal floating point type. I don't see any reason why they have to be the
> same type.

op is just PLUS_EXPR (or another operation), it isn't related to @0, it 
does not have a type in itself. When you build the sum of 2 objects of 
type newtype, the result has type newtype. On the other hand, if newtype 
is not the same as type, you may be missing a conversion of the result to 
type. Ah, I see that newtype is always == type here.

>>> +	    (nop:type (op (convert:ty1 @1) (convert:ty2 @2)))))
>>
>> Please don't use 'nop' directly, use 'convert' instead. This line is very
>> suspicious, both arguments of op should have the same type. Specifying the
>> outertype should be unnecessary, it is always 'type'. And if necessary, I
>> expect '(convert:ty1 @1)' is the same as '{ arg0; }'.
>>
>
> Ah I wasn't aware I could use arg0 here. I've updated the patch, though I don't
> really find this clearer.

> + (convert (op (convert:ty1 { arg0; }) (convert:ty2 { arg1; })))))

I think you misunderstood my point. What you wrote is equivalent to:

 	(convert (op { arg0; } { arg1; }))))

since arg0 already has type ty1. And I am complaining that both arguments 
to op must have the same type, but you are creating one of type ty1 and 
one of type ty2, which doesn't clearly indicate that ty1==ty2.

Maybe experiment with
(long double)some_float * (long double)some_double
cast to either float or double.

SCALAR_FLOAT_TYPE_P may be safer than FLOAT_TYPE_P.

-- 
Marc Glisse

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

Hi Marc,

> >>> +	    (nop:type (op (convert:ty1 @1) (convert:ty2 @2)))))
> >>
> >> Please don't use 'nop' directly, use 'convert' instead. This line is very
> >> suspicious, both arguments of op should have the same type. Specifying the
> >> outertype should be unnecessary, it is always 'type'. And if necessary, I
> >> expect '(convert:ty1 @1)' is the same as '{ arg0; }'.
> >>
> >
> > Ah I wasn't aware I could use arg0 here. I've updated the patch, though I don't
> > really find this clearer.
> 
> > + (convert (op (convert:ty1 { arg0; }) (convert:ty2 { arg1; })))))
> 
> I think you misunderstood my point. What you wrote is equivalent to:
> 
>  	(convert (op { arg0; } { arg1; }))))
> 
> since arg0 already has type ty1. And I am complaining that both arguments 
> to op must have the same type, but you are creating one of type ty1 and 
> one of type ty2, which doesn't clearly indicate that ty1==ty2.
>

Ah ok, I've reverted the previous changes and added a types_match on ty1 and ty2.
 
> Maybe experiment with
> (long double)some_float * (long double)some_double
> cast to either float or double.
> 

I did, and none of them were an issue:

All of these worked fine and did the operation as expected as DFmode
now, whereas before all except the first would have used TFmode.

double foo (float a, double b)
{
  return (long double)a * (long double)b;
}

double bar (float a, double b)
{
  float x = (long double)a;
  return x * (long double)b;
}

void foo_ (double b, double *c)
{
  float a = (double)3.0d;
  long double e = (long double)a * (long double)b;
  *c = (double)e;
}

> SCALAR_FLOAT_TYPE_P may be safer than FLOAT_TYPE_P.

Hmm, I can't think of any reason why these rules shouldn't apply to vector or
complex float modes.  The old code in convert.c already used FLOAT_TYPE_P.

I've attached the updated patch.

Thanks,
Tamar

> 
> -- 
> Marc Glisse

-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb10277.patch --]
[-- Type: text/x-diff; name="rb10277.patch", Size: 11518 bytes --]

diff --git a/gcc/convert.c b/gcc/convert.c
index 1a3353c870768a33fe22480ec97c7d3e0c504075..a16b7af0ec54693eb4f1e3a110aabc1aa18eb8df 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -295,92 +295,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 97a94cd8b2f2e0fee9ffbc76c5277c97689b6f42..974a730fdef30cc79bdd45610d2cfd1c42a7623a 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4758,37 +4758,116 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
    term we want to move all that code out of the front-ends into here.  */
 
-/* If we have a narrowing conversion of an arithmetic operation where
-   both operands are widening conversions from the same type as the outer
-   narrowing conversion.  Then convert the innermost operands to a suitable
-   unsigned type (to avoid introducing undefined behavior), perform the
-   operation and convert the result to the desired type.  */
-(for op (plus minus)
-  (simplify
-    (convert (op:s (convert@2 @0) (convert?@3 @1)))
-    (if (INTEGRAL_TYPE_P (type)
-	 /* We check for type compatibility between @0 and @1 below,
-	    so there's no need to check that @1/@3 are integral types.  */
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@2))
-	 /* The precision of the type of each operand must match the
-	    precision of the mode of each operand, similarly for the
-	    result.  */
-	 && type_has_mode_precision_p (TREE_TYPE (@0))
-	 && type_has_mode_precision_p (TREE_TYPE (@1))
-	 && type_has_mode_precision_p (type)
-	 /* The inner conversion must be a widening conversion.  */
-	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
-	 && types_match (@0, type)
-	 && (types_match (@0, @1)
-	     /* Or the second operand is const integer or converted const
-		integer from valueize.  */
-	     || TREE_CODE (@1) == INTEGER_CST))
-      (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
-	(op @0 (convert @1))
-	(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-	 (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?@3 @1) (convert2?@4 @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); }
+    (switch
+      (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))
+	      && types_match (ty1, ty2))
+	    (convert (op (convert:ty1 @1) (convert:ty2 @2)))))
+
+      /* If we have a narrowing conversion of an arithmetic operation where
+	 both operands are widening conversions from the same type as the outer
+	 narrowing conversion.  Then convert the innermost operands to a
+	 suitable unsigned type (to avoid introducing undefined behavior),
+	 perform the operation and convert the result to the desired type.  */
+      (if (INTEGRAL_TYPE_P (type)
+	   && op != MULT_EXPR
+	   && op != RDIV_EXPR
+	   /* We check for type compatibility between @0 and @1 below,
+	      so there's no need to check that @2/@4 are integral types.  */
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@3))
+	   /* The precision of the type of each operand must match the
+	      precision of the mode of each operand, similarly for the
+	      result.  */
+	   && type_has_mode_precision_p (TREE_TYPE (@1))
+	   && type_has_mode_precision_p (TREE_TYPE (@2))
+	   && type_has_mode_precision_p (type)
+	   /* The inner conversion must be a widening conversion.  */
+	   && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (TREE_TYPE (@1))
+	   && types_match (@1, type)
+	   && (types_match (@1, @2)
+	       /* Or the second operand is const integer or converted const
+		  integer from valueize.  */
+	       || TREE_CODE (@2) == INTEGER_CST))
+        (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
+	  (op @1 (convert @2))
+	  (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
+	   (convert (op (convert:utype @1)
+		        (convert:utype @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
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" } } */

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 8067 bytes --]

On Tue, 2 Jul 2019, Tamar Christina wrote:

> 
> Hi All,
> 
> Here's an updated patch with the changes processed from the previous review.
> 
> I've bootstrapped and regtested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu and no issues.
> 
> Ok for trunk?

+     (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
+       (op @1 (convert @2))
+       (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
+       (convert (op (convert:utype @1)

indenting is off, one space more for (convert (it's inside the with)

+                   (convert:utype @2)))))
+      (with { tree arg0 = strip_float_extensions (@1);

indenting is off here, one space less for the (with please.

you'll run into strip_float_extensions for integer types as well so
please move the FLOAT_TYPE_P (type) check before the with like

     (if (FLOAT_TYPE_P (type)
          && DECIMAL_FLOAT_TYPE_P (TREE_TPE (@0)) == DECIMAL_FLOAT_TYPE_P 
(type))
      (with { tree arg0 = strip_float_extensions (@1);

+         (if ((newtype == dfloat32_type_node
+               || newtype == dfloat64_type_node
+               || newtype == dfloat128_type_node)
+             && newtype == type)
+           (convert:newtype (op (convert:newtype @1) (convert:newtype 
@2)))

I think you want to elide the outermost convert:newtype here and use

            (op (convert:newtype @1) (convert:newtype @2))

newtype == type check you also want to write types_match_p (newtype, type)

+             (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)))

here the outermost convert looks bogus - you need to build an
expression of type 'type' thus

   (convert:type (op (convert:newtype @1) (convert:newtype @2)))

I think you also want to avoid endless recursion by adding a

             && !types_match_p (itype, newtype)

since in that case you've re-created the original expression.

OK with those changes.

Thanks,
Richard.

> Thanks,
> Tamar
> 
> The 07/02/2019 11:20, Richard Biener wrote:
> > On Tue, 2 Jul 2019, Tamar Christina wrote:
> > 
> > > Hi Richi,
> > > 
> > > The 06/25/2019 10:02, Richard Biener wrote:
> > > > 
> > > > This looks like a literal 1:1 translation plus merging with the
> > > > existing pattern around integers.  You changed
> > > > (op:s@0 (convert@3 @1) (convert?@4 @2)) to
> > > > (op:s@0 (convert1?@3 @1) (convert2?@4 @2)) where this now also
> > > > matches if there are no inner conversions at all - was that a
> > > > desired change or did you merely want to catch when the first
> > > > operand is not a conversion but the second is, possibly only
> > > > for the RDIV_EXPR case?
> > > >
> > > 
> > > Yes, the ? ? is for the RDIV case, I really only want (c a) `op` (c b),
> > > a `op` (c b) and (c a) `op` b.  But didn't find a way to do this.
> > 
> > One way would be to do
> > 
> >  (simplify
> >   (convert (op:sc@0 (convert @1) (convert? @2)))
> > 
> > but that doesn't work for RDIV.  Using :C is tempting but you
> > do not get to know the original operand order which you of
> > course need.  So I guess the way you do it is fine - you
> > could guard all of the code with a few types_match () checks
> > but I'm not sure it is worth the trouble.
> > 
> > Richard.
> > 
> > > The only thing I can think of that gets this is without overmatching is
> > > to either duplicate the code or move this back to a C helper function and
> > > call that from match.pd.  But I was hoping to have it all in match.pd
> > > instead of hiding it away in a C call.
> > > 
> > > Do you have a better way of doing it or a preference to an approach?
> > >  
> > > > +(for op (plus minus mult rdiv)
> > > > + (simplify
> > > > +   (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
> > > > +   (with { tree arg0 = strip_float_extensions (@1);
> > > > +          tree arg1 = strip_float_extensions (@2);
> > > > +          tree itype = TREE_TYPE (@0);
> > > > 
> > > > you now unconditionally call strip_float_extensions on each operand
> > > > even for the integer case, please sink stuff only used in one
> > > > case arm.  I guess keeping the integer case first via
> > > > 
> > > 
> > > Done, Initially didn't think it would be an issue since I don't use the value it
> > > creates in the integer case. But I re-ordered it.
> > >  
> > > > should work (with the 'with' being in the ifs else position).
> > > > 
> > > > +      (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))
> > > > +             && types_match (ty1, ty2))
> > > > +           (convert (op (convert:ty1 @1) (convert:ty2 @2)))))
> > > > 
> > > > this looks prone to the same recursion issue you described above.
> > > 
> > > It's to break the recursion when you don't match anything. Indeed don't need it if
> > > I change the match condition above.
> > > Thanks,
> > > Tamar
> > > > 
> > > > 'code' is used exactly once, using SCALAR_FLOAT_TYPE_P (itype)
> > > > in the above test would be clearer.  Also both ifs can be combined.
> > > > The snipped above also doesn't appear in the convert.c code you
> > > > remove and the original one is
> > > > 
> > > >   switch (TREE_CODE (TREE_TYPE (expr)))
> > > >     {
> > > >     case REAL_TYPE:
> > > >       /* Ignore the conversion if we don't need to store intermediate
> > > >          results and neither type is a decimal float.  */
> > > >       return build1_loc (loc,
> > > >                          (flag_float_store
> > > >                           || DECIMAL_FLOAT_TYPE_P (type)
> > > >                           || DECIMAL_FLOAT_TYPE_P (itype))
> > > >                          ? CONVERT_EXPR : NOP_EXPR, type, expr);
> > > > 
> > > > which as far as I can see doesn't do anything besides
> > > > exchanging CONVERT_EXPR for NOP_EXPR which is a noop to the IL.
> > > > So it appears this shouldn't be moved to match.pd at all?
> > > > It's also not a 1:1 move since you are changing 'expr'.
> > > > 
> > > > Thanks,
> > > > Richard.
> > > > 
> > > > > > 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:
> > > > > > 
> > > > > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > > > > 
> > > > > > 	* convert.c (convert_to_real_1): Move part of conversion code...
> > > > > > 	* match.pd: ...To here.
> > > > > > 
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > 
> > > > > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > > > > 
> > > > > > 	* gcc.dg/type-convert-var.c: New test.
> > > > > > 
> > > > > > --
> > > > > 
> > > > 
> > > > -- 
> > > > Richard Biener <rguenther@suse.de>
> > > > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> > > > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
> > > 
> > > 
> > 
> > -- 
> > Richard Biener <rguenther@suse.de>
> > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 5584 bytes --]


Hi All,

Here's an updated patch with the changes processed from the previous review.

I've bootstrapped and regtested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu and no issues.

Ok for trunk?

Thanks,
Tamar

The 07/02/2019 11:20, Richard Biener wrote:
> On Tue, 2 Jul 2019, Tamar Christina wrote:
> 
> > Hi Richi,
> > 
> > The 06/25/2019 10:02, Richard Biener wrote:
> > > 
> > > This looks like a literal 1:1 translation plus merging with the
> > > existing pattern around integers.  You changed
> > > (op:s@0 (convert@3 @1) (convert?@4 @2)) to
> > > (op:s@0 (convert1?@3 @1) (convert2?@4 @2)) where this now also
> > > matches if there are no inner conversions at all - was that a
> > > desired change or did you merely want to catch when the first
> > > operand is not a conversion but the second is, possibly only
> > > for the RDIV_EXPR case?
> > >
> > 
> > Yes, the ? ? is for the RDIV case, I really only want (c a) `op` (c b),
> > a `op` (c b) and (c a) `op` b.  But didn't find a way to do this.
> 
> One way would be to do
> 
>  (simplify
>   (convert (op:sc@0 (convert @1) (convert? @2)))
> 
> but that doesn't work for RDIV.  Using :C is tempting but you
> do not get to know the original operand order which you of
> course need.  So I guess the way you do it is fine - you
> could guard all of the code with a few types_match () checks
> but I'm not sure it is worth the trouble.
> 
> Richard.
> 
> > The only thing I can think of that gets this is without overmatching is
> > to either duplicate the code or move this back to a C helper function and
> > call that from match.pd.  But I was hoping to have it all in match.pd
> > instead of hiding it away in a C call.
> > 
> > Do you have a better way of doing it or a preference to an approach?
> >  
> > > +(for op (plus minus mult rdiv)
> > > + (simplify
> > > +   (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
> > > +   (with { tree arg0 = strip_float_extensions (@1);
> > > +          tree arg1 = strip_float_extensions (@2);
> > > +          tree itype = TREE_TYPE (@0);
> > > 
> > > you now unconditionally call strip_float_extensions on each operand
> > > even for the integer case, please sink stuff only used in one
> > > case arm.  I guess keeping the integer case first via
> > > 
> > 
> > Done, Initially didn't think it would be an issue since I don't use the value it
> > creates in the integer case. But I re-ordered it.
> >  
> > > should work (with the 'with' being in the ifs else position).
> > > 
> > > +      (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))
> > > +             && types_match (ty1, ty2))
> > > +           (convert (op (convert:ty1 @1) (convert:ty2 @2)))))
> > > 
> > > this looks prone to the same recursion issue you described above.
> > 
> > It's to break the recursion when you don't match anything. Indeed don't need it if
> > I change the match condition above.
> > Thanks,
> > Tamar
> > > 
> > > 'code' is used exactly once, using SCALAR_FLOAT_TYPE_P (itype)
> > > in the above test would be clearer.  Also both ifs can be combined.
> > > The snipped above also doesn't appear in the convert.c code you
> > > remove and the original one is
> > > 
> > >   switch (TREE_CODE (TREE_TYPE (expr)))
> > >     {
> > >     case REAL_TYPE:
> > >       /* Ignore the conversion if we don't need to store intermediate
> > >          results and neither type is a decimal float.  */
> > >       return build1_loc (loc,
> > >                          (flag_float_store
> > >                           || DECIMAL_FLOAT_TYPE_P (type)
> > >                           || DECIMAL_FLOAT_TYPE_P (itype))
> > >                          ? CONVERT_EXPR : NOP_EXPR, type, expr);
> > > 
> > > which as far as I can see doesn't do anything besides
> > > exchanging CONVERT_EXPR for NOP_EXPR which is a noop to the IL.
> > > So it appears this shouldn't be moved to match.pd at all?
> > > It's also not a 1:1 move since you are changing 'expr'.
> > > 
> > > Thanks,
> > > Richard.
> > > 
> > > > > 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:
> > > > > 
> > > > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > > > 
> > > > > 	* convert.c (convert_to_real_1): Move part of conversion code...
> > > > > 	* match.pd: ...To here.
> > > > > 
> > > > > gcc/testsuite/ChangeLog:
> > > > > 
> > > > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > > > 
> > > > > 	* gcc.dg/type-convert-var.c: New test.
> > > > > 
> > > > > --
> > > > 
> > > 
> > > -- 
> > > Richard Biener <rguenther@suse.de>
> > > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> > > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb10277.patch --]
[-- Type: text/x-diff; name="rb10277.patch", Size: 11102 bytes --]

diff --git a/gcc/convert.c b/gcc/convert.c
index a8f2bd049ba0cd83865ba0a5f7d74f9cdbad0d09..7f0d933f4d9e29719acb27eb1b32a9e540d93073 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -298,92 +298,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 f8e35e96d22036bb0b96fbdbe2c7a346f4695067..66a1ad385ffff4456c87a8891ab78c437fefc64f 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4937,37 +4937,106 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
    term we want to move all that code out of the front-ends into here.  */
 
-/* If we have a narrowing conversion of an arithmetic operation where
-   both operands are widening conversions from the same type as the outer
-   narrowing conversion.  Then convert the innermost operands to a suitable
-   unsigned type (to avoid introducing undefined behavior), perform the
-   operation and convert the result to the desired type.  */
-(for op (plus minus)
-  (simplify
-    (convert (op:s (convert@2 @0) (convert?@3 @1)))
-    (if (INTEGRAL_TYPE_P (type)
-	 /* We check for type compatibility between @0 and @1 below,
-	    so there's no need to check that @1/@3 are integral types.  */
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@2))
-	 /* The precision of the type of each operand must match the
-	    precision of the mode of each operand, similarly for the
-	    result.  */
-	 && type_has_mode_precision_p (TREE_TYPE (@0))
-	 && type_has_mode_precision_p (TREE_TYPE (@1))
-	 && type_has_mode_precision_p (type)
-	 /* The inner conversion must be a widening conversion.  */
-	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
-	 && types_match (@0, type)
-	 && (types_match (@0, @1)
-	     /* Or the second operand is const integer or converted const
-		integer from valueize.  */
-	     || TREE_CODE (@1) == INTEGER_CST))
-      (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
-	(op @0 (convert @1))
-	(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-	 (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?@3 @1) (convert2?@4 @2)))
+   /* If we have a narrowing conversion of an arithmetic operation where
+      both operands are widening conversions from the same type as the outer
+      narrowing conversion.  Then convert the innermost operands to a
+      suitable unsigned type (to avoid introducing undefined behavior),
+      perform the operation and convert the result to the desired type.  */
+   (if (INTEGRAL_TYPE_P (type)
+	&& op != MULT_EXPR
+	&& op != RDIV_EXPR
+	/* We check for type compatibility between @0 and @1 below,
+	   so there's no need to check that @2/@4 are integral types.  */
+	&& INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	&& INTEGRAL_TYPE_P (TREE_TYPE (@3))
+	/* The precision of the type of each operand must match the
+	   precision of the mode of each operand, similarly for the
+	   result.  */
+	&& type_has_mode_precision_p (TREE_TYPE (@1))
+	&& type_has_mode_precision_p (TREE_TYPE (@2))
+	&& type_has_mode_precision_p (type)
+	/* The inner conversion must be a widening conversion.  */
+	&& TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (TREE_TYPE (@1))
+	&& types_match (@1, type)
+	&& (types_match (@1, @2)
+	    /* Or the second operand is const integer or converted const
+	       integer from valueize.  */
+	    || TREE_CODE (@2) == INTEGER_CST))
+     (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
+       (op @1 (convert @2))
+       (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
+       (convert (op (convert:utype @1)
+		    (convert:utype @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)))
+	 )))) )
+   )
+)))
 
 /* 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
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" } } */

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 4897 bytes --]

On Tue, 2 Jul 2019, Tamar Christina wrote:

> Hi Richi,
> 
> The 06/25/2019 10:02, Richard Biener wrote:
> > 
> > This looks like a literal 1:1 translation plus merging with the
> > existing pattern around integers.  You changed
> > (op:s@0 (convert@3 @1) (convert?@4 @2)) to
> > (op:s@0 (convert1?@3 @1) (convert2?@4 @2)) where this now also
> > matches if there are no inner conversions at all - was that a
> > desired change or did you merely want to catch when the first
> > operand is not a conversion but the second is, possibly only
> > for the RDIV_EXPR case?
> >
> 
> Yes, the ? ? is for the RDIV case, I really only want (c a) `op` (c b),
> a `op` (c b) and (c a) `op` b.  But didn't find a way to do this.

One way would be to do

 (simplify
  (convert (op:sc@0 (convert @1) (convert? @2)))

but that doesn't work for RDIV.  Using :C is tempting but you
do not get to know the original operand order which you of
course need.  So I guess the way you do it is fine - you
could guard all of the code with a few types_match () checks
but I'm not sure it is worth the trouble.

Richard.

> The only thing I can think of that gets this is without overmatching is
> to either duplicate the code or move this back to a C helper function and
> call that from match.pd.  But I was hoping to have it all in match.pd
> instead of hiding it away in a C call.
> 
> Do you have a better way of doing it or a preference to an approach?
>  
> > +(for op (plus minus mult rdiv)
> > + (simplify
> > +   (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
> > +   (with { tree arg0 = strip_float_extensions (@1);
> > +          tree arg1 = strip_float_extensions (@2);
> > +          tree itype = TREE_TYPE (@0);
> > 
> > you now unconditionally call strip_float_extensions on each operand
> > even for the integer case, please sink stuff only used in one
> > case arm.  I guess keeping the integer case first via
> > 
> 
> Done, Initially didn't think it would be an issue since I don't use the value it
> creates in the integer case. But I re-ordered it.
>  
> > should work (with the 'with' being in the ifs else position).
> > 
> > +      (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))
> > +             && types_match (ty1, ty2))
> > +           (convert (op (convert:ty1 @1) (convert:ty2 @2)))))
> > 
> > this looks prone to the same recursion issue you described above.
> 
> It's to break the recursion when you don't match anything. Indeed don't need it if
> I change the match condition above.
> Thanks,
> Tamar
> > 
> > 'code' is used exactly once, using SCALAR_FLOAT_TYPE_P (itype)
> > in the above test would be clearer.  Also both ifs can be combined.
> > The snipped above also doesn't appear in the convert.c code you
> > remove and the original one is
> > 
> >   switch (TREE_CODE (TREE_TYPE (expr)))
> >     {
> >     case REAL_TYPE:
> >       /* Ignore the conversion if we don't need to store intermediate
> >          results and neither type is a decimal float.  */
> >       return build1_loc (loc,
> >                          (flag_float_store
> >                           || DECIMAL_FLOAT_TYPE_P (type)
> >                           || DECIMAL_FLOAT_TYPE_P (itype))
> >                          ? CONVERT_EXPR : NOP_EXPR, type, expr);
> > 
> > which as far as I can see doesn't do anything besides
> > exchanging CONVERT_EXPR for NOP_EXPR which is a noop to the IL.
> > So it appears this shouldn't be moved to match.pd at all?
> > It's also not a 1:1 move since you are changing 'expr'.
> > 
> > Thanks,
> > Richard.
> > 
> > > > 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:
> > > > 
> > > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > > 
> > > > 	* convert.c (convert_to_real_1): Move part of conversion code...
> > > > 	* match.pd: ...To here.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > > 
> > > > 	* gcc.dg/type-convert-var.c: New test.
> > > > 
> > > > --
> > > 
> > 
> > -- 
> > Richard Biener <rguenther@suse.de>
> > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

Hi Richi,

The 06/25/2019 10:02, Richard Biener wrote:
> 
> This looks like a literal 1:1 translation plus merging with the
> existing pattern around integers.  You changed
> (op:s@0 (convert@3 @1) (convert?@4 @2)) to
> (op:s@0 (convert1?@3 @1) (convert2?@4 @2)) where this now also
> matches if there are no inner conversions at all - was that a
> desired change or did you merely want to catch when the first
> operand is not a conversion but the second is, possibly only
> for the RDIV_EXPR case?
>

Yes, the ? ? is for the RDIV case, I really only want (c a) `op` (c b),
a `op` (c b) and (c a) `op` b.  But didn't find a way to do this.

The only thing I can think of that gets this is without overmatching is
to either duplicate the code or move this back to a C helper function and
call that from match.pd.  But I was hoping to have it all in match.pd
instead of hiding it away in a C call.

Do you have a better way of doing it or a preference to an approach?
 
> +(for op (plus minus mult rdiv)
> + (simplify
> +   (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
> +   (with { tree arg0 = strip_float_extensions (@1);
> +          tree arg1 = strip_float_extensions (@2);
> +          tree itype = TREE_TYPE (@0);
> 
> you now unconditionally call strip_float_extensions on each operand
> even for the integer case, please sink stuff only used in one
> case arm.  I guess keeping the integer case first via
> 

Done, Initially didn't think it would be an issue since I don't use the value it
creates in the integer case. But I re-ordered it.
 
> should work (with the 'with' being in the ifs else position).
> 
> +      (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))
> +             && types_match (ty1, ty2))
> +           (convert (op (convert:ty1 @1) (convert:ty2 @2)))))
> 
> this looks prone to the same recursion issue you described above.

It's to break the recursion when you don't match anything. Indeed don't need it if
I change the match condition above.

Thanks,
Tamar
> 
> 'code' is used exactly once, using SCALAR_FLOAT_TYPE_P (itype)
> in the above test would be clearer.  Also both ifs can be combined.
> The snipped above also doesn't appear in the convert.c code you
> remove and the original one is
> 
>   switch (TREE_CODE (TREE_TYPE (expr)))
>     {
>     case REAL_TYPE:
>       /* Ignore the conversion if we don't need to store intermediate
>          results and neither type is a decimal float.  */
>       return build1_loc (loc,
>                          (flag_float_store
>                           || DECIMAL_FLOAT_TYPE_P (type)
>                           || DECIMAL_FLOAT_TYPE_P (itype))
>                          ? CONVERT_EXPR : NOP_EXPR, type, expr);
> 
> which as far as I can see doesn't do anything besides
> exchanging CONVERT_EXPR for NOP_EXPR which is a noop to the IL.
> So it appears this shouldn't be moved to match.pd at all?
> It's also not a 1:1 move since you are changing 'expr'.
> 
> Thanks,
> Richard.
> 
> > > 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:
> > > 
> > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > 
> > > 	* convert.c (convert_to_real_1): Move part of conversion code...
> > > 	* match.pd: ...To here.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > > 
> > > 	* gcc.dg/type-convert-var.c: New test.
> > > 
> > > --
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

-- 

RE: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 5024 bytes --]

On Tue, 25 Jun 2019, Tamar Christina wrote:

> Adding some maintainers
> 
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> On 
> > Behalf Of Tamar Christina
> > Sent: Tuesday, June 25, 2019 09:31
> > To: gcc-patches@gcc.gnu.org
> > Cc: nd <nd@arm.com>
> > Subject: [GCC][middle-end] Add rules to strip away unneeded type casts 
> > in expressions (2nd patch)
> > 
> > Hi All,
> > 
> > This is an updated version of my GCC-9 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.
> > 
> > The previous patch was only regtested on aarch64-none-linux-gnu and I 
> > hadn't done a regression on x86_64-pc-linux-gnu only a bootstrap.  The 
> > previous patch was approved
> > 
> > here https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00116.html
> > but before committing I ran a x86_64-pc-linux-gnu regtest to be sure 
> > and this showed an issue with a DFP test. I Have fixed this by 
> > removing the offending convert.
> > The convert was just saying "keep the type as is" but match.pd looped 
> > here as it thinks the match did something and would try other 
> > patterns, causing it to match itself again.
> > 
> > Instead when there's nothing to update, I just don't do anything.
> > 
> > The second change was to merge this with the existing pattern for 
> > integer conversion in order to silence a warning from match.pd which 
> > though that the two patterns overlaps because their match conditions 
> > are similar (they have different conditions inside the ifs but 
> > match.pd doesn't check those of course.).
> > 
> > Regtested and bootstrapped on aarch64-none-linux-gnu and x86_64-pc- 
> > linux-gnu and no issues.
> > 
> > Ok for trunk?

This looks like a literal 1:1 translation plus merging with the
existing pattern around integers.  You changed
(op:s@0 (convert@3 @1) (convert?@4 @2)) to
(op:s@0 (convert1?@3 @1) (convert2?@4 @2)) where this now also
matches if there are no inner conversions at all - was that a
desired change or did you merely want to catch when the first
operand is not a conversion but the second is, possibly only
for the RDIV_EXPR case?

+(for op (plus minus mult rdiv)
+ (simplify
+   (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
+   (with { tree arg0 = strip_float_extensions (@1);
+          tree arg1 = strip_float_extensions (@2);
+          tree itype = TREE_TYPE (@0);

you now unconditionally call strip_float_extensions on each operand
even for the integer case, please sink stuff only used in one
case arm.  I guess keeping the integer case first via

  (if (INTEGRAL_TYPE_P (type)
...
   (with { tree arg0 = strip_float_extensions (@1);
...

should work (with the 'with' being in the ifs else position).

+      (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))
+             && types_match (ty1, ty2))
+           (convert (op (convert:ty1 @1) (convert:ty2 @2)))))

this looks prone to the same recursion issue you described above.

'code' is used exactly once, using SCALAR_FLOAT_TYPE_P (itype)
in the above test would be clearer.  Also both ifs can be combined.
The snipped above also doesn't appear in the convert.c code you
remove and the original one is

  switch (TREE_CODE (TREE_TYPE (expr)))
    {
    case REAL_TYPE:
      /* Ignore the conversion if we don't need to store intermediate
         results and neither type is a decimal float.  */
      return build1_loc (loc,
                         (flag_float_store
                          || DECIMAL_FLOAT_TYPE_P (type)
                          || DECIMAL_FLOAT_TYPE_P (itype))
                         ? CONVERT_EXPR : NOP_EXPR, type, expr);

which as far as I can see doesn't do anything besides
exchanging CONVERT_EXPR for NOP_EXPR which is a noop to the IL.
So it appears this shouldn't be moved to match.pd at all?
It's also not a 1:1 move since you are changing 'expr'.

Thanks,
Richard.

> > 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:
> > 
> > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > 
> > 	* convert.c (convert_to_real_1): Move part of conversion code...
> > 	* match.pd: ...To here.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> > 
> > 	* gcc.dg/type-convert-var.c: New test.
> > 
> > --
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

RE: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

Adding some maintainers

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> On 
> Behalf Of Tamar Christina
> Sent: Tuesday, June 25, 2019 09:31
> To: gcc-patches@gcc.gnu.org
> Cc: nd <nd@arm.com>
> Subject: [GCC][middle-end] Add rules to strip away unneeded type casts 
> in expressions (2nd patch)
> 
> Hi All,
> 
> This is an updated version of my GCC-9 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.
> 
> The previous patch was only regtested on aarch64-none-linux-gnu and I 
> hadn't done a regression on x86_64-pc-linux-gnu only a bootstrap.  The 
> previous patch was approved
> 
> here https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00116.html
> but before committing I ran a x86_64-pc-linux-gnu regtest to be sure 
> and this showed an issue with a DFP test. I Have fixed this by 
> removing the offending convert.
> The convert was just saying "keep the type as is" but match.pd looped 
> here as it thinks the match did something and would try other 
> patterns, causing it to match itself again.
> 
> Instead when there's nothing to update, I just don't do anything.
> 
> The second change was to merge this with the existing pattern for 
> integer conversion in order to silence a warning from match.pd which 
> though that the two patterns overlaps because their match conditions 
> are similar (they have different conditions inside the ifs but 
> match.pd doesn't check those of course.).
> 
> Regtested and bootstrapped on aarch64-none-linux-gnu and x86_64-pc- 
> 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:
> 
> 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* convert.c (convert_to_real_1): Move part of conversion code...
> 	* match.pd: ...To here.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-06-25  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* gcc.dg/type-convert-var.c: New test.
> 
> --

[GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

Hi All,

This is an updated version of my GCC-9 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.

The previous patch was only regtested on aarch64-none-linux-gnu and I hadn't done a
regression on x86_64-pc-linux-gnu only a bootstrap.  The previous patch was approved

here https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00116.html
but before committing I ran a x86_64-pc-linux-gnu regtest to be sure and this
showed an issue with a DFP test. I Have fixed this by removing the offending convert.
The convert was just saying "keep the type as is" but match.pd looped here as it thinks
the match did something and would try other patterns, causing it to match itself again.

Instead when there's nothing to update, I just don't do anything.

The second change was to merge this with the existing pattern for integer conversion
in order to silence a warning from match.pd which though that the two patterns overlaps
because their match conditions are similar (they have different conditions inside the ifs
but match.pd doesn't check those of course.).

Regtested and bootstrapped on aarch64-none-linux-gnu and x86_64-pc-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:

2019-06-25  Tamar Christina  <tamar.christina@arm.com>

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

gcc/testsuite/ChangeLog:

2019-06-25  Tamar Christina  <tamar.christina@arm.com>

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

-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb10277.patch --]
[-- Type: text/x-diff; name="rb10277.patch", Size: 11518 bytes --]

diff --git a/gcc/convert.c b/gcc/convert.c
index d5aa07b510e0e7831e8d121b383e42e5c6e89321..923eb70366e6c05141fb1580ba6f85e354aa3f76 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -298,92 +298,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 f9bc097c49122bf1b4bcf0b12b09840daf7b8fbc..228a69f99ae0318f034b2f713e522acd9e0995ae 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4870,37 +4870,116 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
    term we want to move all that code out of the front-ends into here.  */
 
-/* If we have a narrowing conversion of an arithmetic operation where
-   both operands are widening conversions from the same type as the outer
-   narrowing conversion.  Then convert the innermost operands to a suitable
-   unsigned type (to avoid introducing undefined behavior), perform the
-   operation and convert the result to the desired type.  */
-(for op (plus minus)
-  (simplify
-    (convert (op:s (convert@2 @0) (convert?@3 @1)))
-    (if (INTEGRAL_TYPE_P (type)
-	 /* We check for type compatibility between @0 and @1 below,
-	    so there's no need to check that @1/@3 are integral types.  */
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	 && INTEGRAL_TYPE_P (TREE_TYPE (@2))
-	 /* The precision of the type of each operand must match the
-	    precision of the mode of each operand, similarly for the
-	    result.  */
-	 && type_has_mode_precision_p (TREE_TYPE (@0))
-	 && type_has_mode_precision_p (TREE_TYPE (@1))
-	 && type_has_mode_precision_p (type)
-	 /* The inner conversion must be a widening conversion.  */
-	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
-	 && types_match (@0, type)
-	 && (types_match (@0, @1)
-	     /* Or the second operand is const integer or converted const
-		integer from valueize.  */
-	     || TREE_CODE (@1) == INTEGER_CST))
-      (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
-	(op @0 (convert @1))
-	(with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
-	 (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?@3 @1) (convert2?@4 @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); }
+    (switch
+      (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))
+	      && types_match (ty1, ty2))
+	    (convert (op (convert:ty1 @1) (convert:ty2 @2)))))
+
+      /* If we have a narrowing conversion of an arithmetic operation where
+	 both operands are widening conversions from the same type as the outer
+	 narrowing conversion.  Then convert the innermost operands to a
+	 suitable unsigned type (to avoid introducing undefined behavior),
+	 perform the operation and convert the result to the desired type.  */
+      (if (INTEGRAL_TYPE_P (type)
+	   && op != MULT_EXPR
+	   && op != RDIV_EXPR
+	   /* We check for type compatibility between @0 and @1 below,
+	      so there's no need to check that @2/@4 are integral types.  */
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	   && INTEGRAL_TYPE_P (TREE_TYPE (@3))
+	   /* The precision of the type of each operand must match the
+	      precision of the mode of each operand, similarly for the
+	      result.  */
+	   && type_has_mode_precision_p (TREE_TYPE (@1))
+	   && type_has_mode_precision_p (TREE_TYPE (@2))
+	   && type_has_mode_precision_p (type)
+	   /* The inner conversion must be a widening conversion.  */
+	   && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (TREE_TYPE (@1))
+	   && types_match (@1, type)
+	   && (types_match (@1, @2)
+	       /* Or the second operand is const integer or converted const
+		  integer from valueize.  */
+	       || TREE_CODE (@2) == INTEGER_CST))
+        (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
+	  (op @1 (convert @2))
+	  (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
+	   (convert (op (convert:utype @1)
+		        (convert:utype @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
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" } } */