public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tamar Christina <Tamar.Christina@arm.com>
To: "gcc-patches@gcc.gnu.org" <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)
Date: Tue, 25 Jun 2019 08:31:00 -0000	[thread overview]
Message-ID: <20190625083126.GA19632@arm.com> (raw)

[-- 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" } } */


             reply	other threads:[~2019-06-25  8:31 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-25  8:31 Tamar Christina [this message]
2019-06-25  8:33 ` Tamar Christina
2019-06-25  9:02   ` Richard Biener
2019-07-02  9:41     ` Tamar Christina
2019-07-02 10:20       ` Richard Biener
2019-07-02 16:44         ` Tamar Christina
2019-07-03  9:06           ` Richard Biener
2019-07-30  7:05           ` [PATCH] Fix up gcc.dg/type-convert-var.c testcase (was Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)) Jakub Jelinek
2019-07-30  7:09             ` Rainer Orth
2019-07-30  7:13               ` Jakub Jelinek
2019-07-30  7:34                 ` Richard Biener
2019-07-30  7:11           ` Fix up -fexcess-precision handling in LTO " Jakub Jelinek
2019-07-30  7:42             ` Richard Biener
2019-07-30  7:46               ` Jakub Jelinek
2019-08-06 15:02             ` Patch ping (Re: Fix up -fexcess-precision handling in LTO (was Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch))) Jakub Jelinek
2019-08-19 22:27             ` Fix up -fexcess-precision handling in LTO (was Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)) Joseph Myers
  -- strict thread matches above, loose matches on Subject: below --
2019-01-04 10:46 [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch) Tamar Christina
2019-01-04 17:51 ` Marc Glisse
2019-01-07 14:32   ` Tamar Christina
2019-01-07 14:56     ` Marc Glisse
2019-01-08 13:57       ` Tamar Christina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190625083126.GA19632@arm.com \
    --to=tamar.christina@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).