Re: Fix PR57886, invalid folding of conversion
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.
Re: Fix PR57886, invalid folding of conversion
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
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
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; +}
Fix PR57886, invalid folding of conversion
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; +}