Fix PR57886, invalid folding of conversion

Fix PR57886, invalid folding of conversion

[Michael Matz]

Hi,

GCC happily transforms (float)-z into -(float)z, even when z is of 
unsigned type (when it's larger than float).  That's wrong (the result 
should always be positive, because -z is).  It seems to me that this bug 
exists in all reasonably recent GCC versions.  The checking in 
convert_to_real is wrong, it compares type precisions of different 
classed types.  The change with the least impact is to reject only 
TYPE_UNSIGNED inner types, but perhaps it would be better to only do the 
transformation if the inner type is FLOAT_TYPE_P as well.

Regstrapping on x86-64 in progress.  Okay for trunk?


Ciao,
Michael.
	PR middle-end/57886
	* convert.c (convert_to_real): Reject unsigned inner types.

testsuite/
	* c-c++-common/pr57886.c: New test.

Index: convert.c
===================================================================
--- convert.c	(revision 200240)
+++ convert.c	(working copy)
@@ -213,10 +213,11 @@ convert_to_real (tree type, tree expr)
     switch (TREE_CODE (expr))
       {
 	/* Convert (float)-x into -(float)x.  This is safe for
-	   round-to-nearest rounding mode.  */
+	   round-to-nearest rounding mode when the inner type is signed.  */
 	case ABS_EXPR:
 	case NEGATE_EXPR:
 	  if (!flag_rounding_math
+	      && !TYPE_UNSIGNED (itype)
 	      && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (expr)))
 	    return build1 (TREE_CODE (expr), type,
 			   fold (convert_to_real (type,
Index: testsuite/c-c++-common/pr57886.c
===================================================================
--- testsuite/c-c++-common/pr57886.c	(revision 0)
+++ testsuite/c-c++-common/pr57886.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do run } */
+
+float global;
+int main()
+{
+  unsigned long z = 1;
+  float x = -z;
+  global = x;
+  if (global < 0)
+    __builtin_abort ();
+  return 0;
+}

Re: Fix PR57886, invalid folding of conversion

[Michael Matz]

Hi,

On Fri, 12 Jul 2013, Michael Matz wrote:

> Hi,
> 
> GCC happily transforms (float)-z into -(float)z, even when z is of 
> unsigned type (when it's larger than float).  That's wrong (the result 
> should always be positive, because -z is).  It seems to me that this bug 
> exists in all reasonably recent GCC versions.  The checking in 
> convert_to_real is wrong, it compares type precisions of different 
> classed types.  The change with the least impact is to reject only 
> TYPE_UNSIGNED inner types, but perhaps it would be better to only do the 
> transformation if the inner type is FLOAT_TYPE_P as well.

So, this is a dup of PR55771, I'm now proposing to add a test for 
FLOAT_TYPE_P, not TYPE_UNSIGNED.  Restarted the regstrap.


Ciao,
Michael.
-- 
	PR middle-end/55771
	* convert.c (convert_to_real): Reject non-float inner types.

testsuite/
	* c-c++-common/pr55771.c: New test.

Index: convert.c
===================================================================
--- convert.c	(revision 200240)
+++ convert.c	(working copy)
@@ -213,10 +213,11 @@ convert_to_real (tree type, tree expr)
     switch (TREE_CODE (expr))
       {
 	/* Convert (float)-x into -(float)x.  This is safe for
-	   round-to-nearest rounding mode.  */
+	   round-to-nearest rounding mode when the inner type is float.  */
 	case ABS_EXPR:
 	case NEGATE_EXPR:
 	  if (!flag_rounding_math
+	      && FLOAT_TYPE_P (itype)
 	      && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (expr)))
 	    return build1 (TREE_CODE (expr), type,
 			   fold (convert_to_real (type,
Index: testsuite/c-c++-common/pr55771.c
===================================================================
--- testsuite/c-c++-common/pr55771.c	(revision 0)
+++ testsuite/c-c++-common/pr55771.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do run } */
+
+float global;
+int main()
+{
+  unsigned long z = 1;
+  float x = -z;
+  global = x;
+  if (global < 0)
+    __builtin_abort ();
+  return 0;
+}

Re: Fix PR57886, invalid folding of conversion

[Jakub Jelinek]

On Fri, Jul 12, 2013 at 03:37:44PM +0200, Michael Matz wrote:
> So, this is a dup of PR55771, I'm now proposing to add a test for 
> FLOAT_TYPE_P, not TYPE_UNSIGNED.  Restarted the regstrap.

Ok if it passes bootstrap/regtest.  I'd say we should apply it to
4.8 too, while it might not be a regression (or at least not a recent one),
it is a silent wrong-code issue.  Sorry for forgetting about this PR.

> 	PR middle-end/55771
> 	* convert.c (convert_to_real): Reject non-float inner types.
> 
> testsuite/
> 	* c-c++-common/pr55771.c: New test.

	Jakub

Re: Fix PR57886, invalid folding of conversion

[Marc Glisse]

On Fri, 12 Jul 2013, Michael Matz wrote:

> GCC happily transforms (float)-z into -(float)z, even when z is of
> unsigned type (when it's larger than float).  That's wrong (the result
> should always be positive, because -z is).  It seems to me that this bug
> exists in all reasonably recent GCC versions.  The checking in
> convert_to_real is wrong, it compares type precisions of different
> classed types.  The change with the least impact is to reject only
> TYPE_UNSIGNED inner types, but perhaps it would be better to only do the
> transformation if the inner type is FLOAT_TYPE_P as well.

If you want to handle integers, shouldn't you test TYPE_OVERFLOW_UNDEFINED 
(for LONG_MIN)? FLOAT_TYPE_P does seem safer indeed.

I'd replace TREE_TYPE (expr) with itype on the next line, it is confusing 
to refer to it under 2 different names on 2 consecutive lines.

-- 
Marc Glisse

Re: Fix PR57886, invalid folding of conversion

[Michael Matz]

Hi,

On Fri, 12 Jul 2013, Marc Glisse wrote:

> If you want to handle integers, shouldn't you test TYPE_OVERFLOW_UNDEFINED
> (for LONG_MIN)?

Right, ...

> FLOAT_TYPE_P does seem safer indeed.

... hence this is it now.

> 
> I'd replace TREE_TYPE (expr) with itype on the next line, it is confusing to
> refer to it under 2 different names on 2 consecutive lines.

True, I made that change.  r200926.


Ciao,
Michael.