diff mbox series

middle-end/57245 - honor -frounding-math in real truncation

Message ID 99s854-94r4-8823-6n80-nq081641son@fhfr.qr
State New
Headers show
Series middle-end/57245 - honor -frounding-math in real truncation | expand

Commit Message

Richard Biener Oct. 27, 2021, 1:20 p.m. UTC
The following honors -frounding-math when converting a FP constant
to another FP type.

Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?

I wonder what a good way to test this in a portable way, the bugreport
unfortunately didn't contain something executable and I don't see
much -frounding-math test coverage to copy from.

Thanks,
Richard.

2021-10-27  Richard Biener  <rguenther@suse.de>

	PR middle-end/57245
	* fold-const.c (fold_convert_const_real_from_real): Honor
	-frounding-math if the conversion is not exact.
---
 gcc/fold-const.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jakub Jelinek Oct. 27, 2021, 1:41 p.m. UTC | #1
On Wed, Oct 27, 2021 at 03:20:29PM +0200, Richard Biener via Gcc-patches wrote:
> The following honors -frounding-math when converting a FP constant
> to another FP type.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> 
> I wonder what a good way to test this in a portable way, the bugreport
> unfortunately didn't contain something executable and I don't see
> much -frounding-math test coverage to copy from.

E.g. following tests call fesetround, use fenv effective target etc.:
torture/fp-int-convert-float128-timode-3.c:  fesetround (FE_TOWARDZERO);
torture/fp-int-convert-timode-2.c:  fesetround (FE_DOWNWARD);
torture/fp-int-convert-timode-3.c:  fesetround (FE_UPWARD);
torture/fp-int-convert-timode-4.c:  fesetround (FE_TOWARDZERO);

And the test can just hardcode one or more common float/double etc.
configurations, checked using
__{FLT,DBL}_{DIG,MANT_DIG,RADIX,MIN_EXP,MAX_EXP}__ etc. macros.
Say just test double to float conversions of some specific values assuming
float is IEEE754 single precicion and double is IEEE754 double precision
in all the 4 rounding modes.

> 2021-10-27  Richard Biener  <rguenther@suse.de>
> 
> 	PR middle-end/57245
> 	* fold-const.c (fold_convert_const_real_from_real): Honor
> 	-frounding-math if the conversion is not exact.
> ---
>  gcc/fold-const.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index ff23f12f33c..c7aebf9cc7e 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -2139,6 +2139,12 @@ fold_convert_const_real_from_real (tree type, const_tree arg1)
>        && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
>      return NULL_TREE; 
>  
> +  /* With flag_rounding_math we shuld respect the current rounding mode

s/shuld/should/

> +     unless the conversion is exact.  */
> +  if (HONOR_SIGN_DEPENDENT_ROUNDING (arg1)
> +      && !exact_real_truncate (TYPE_MODE (type), &TREE_REAL_CST (arg1)))
> +    return NULL_TREE;
> +
>    real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
>    t = build_real (type, value);

	Jakub
Richard Biener Oct. 27, 2021, 2:29 p.m. UTC | #2
On Wed, 27 Oct 2021, Jakub Jelinek wrote:

> On Wed, Oct 27, 2021 at 03:20:29PM +0200, Richard Biener via Gcc-patches wrote:
> > The following honors -frounding-math when converting a FP constant
> > to another FP type.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> > 
> > I wonder what a good way to test this in a portable way, the bugreport
> > unfortunately didn't contain something executable and I don't see
> > much -frounding-math test coverage to copy from.
> 
> E.g. following tests call fesetround, use fenv effective target etc.:
> torture/fp-int-convert-float128-timode-3.c:  fesetround (FE_TOWARDZERO);
> torture/fp-int-convert-timode-2.c:  fesetround (FE_DOWNWARD);
> torture/fp-int-convert-timode-3.c:  fesetround (FE_UPWARD);
> torture/fp-int-convert-timode-4.c:  fesetround (FE_TOWARDZERO);
> 
> And the test can just hardcode one or more common float/double etc.
> configurations, checked using
> __{FLT,DBL}_{DIG,MANT_DIG,RADIX,MIN_EXP,MAX_EXP}__ etc. macros.
> Say just test double to float conversions of some specific values assuming
> float is IEEE754 single precicion and double is IEEE754 double precision
> in all the 4 rounding modes.

So something like the following below?  Note I have to fix 
simplify_const_unary_operation to not perform the invalid constant
folding with (not worrying about the exact conversion case - I doubt
any of the constant folding is really relevant on RTL these days,
maybe we should simply punt for all unary float-float ops when either
mode has sign dependent rounding modes)

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index bbbd6b74942..9522a31570e 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -2068,6 +2073,9 @@ simplify_const_unary_operation (enum rtx_code code, 
machine_mode mode,
             and the operand is a signaling NaN.  */
          if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
            return NULL_RTX;
+         /* Or if flag_rounding_math is on.  */
+         if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
+           return NULL_RTX;
          d = real_value_truncate (mode, d);
          break;
        case FLOAT_EXTEND:



/* PR57245 */
/* { dg-do run } */
/* { dg-require-effective-target fenv } */
/* { dg-additional-options "-frounding-math" } */

#include <fenv.h>
#include <stdlib.h>

int
main ()
{
#if __DBL_MANT_DIG__ == 53 && __FLT_MANT_DIG__ == 24
  fesetround (FE_UPWARD);
  float f = 1.3;
  if (f != 0x1.4ccccep+0f)
    __builtin_abort ();
  fesetround (FE_TONEAREST);
  /* Use different actual values so the bogus CSE we perform does not
     break things.  */
  f = 1.33;
  if (f != 0x1.547ae2p+0f)
    abort ();
  fesetround (FE_DOWNWARD);
  f = 1.333;
  if (f != 0x1.553f7cp+0f)
    abort ();
  fesetround (FE_TOWARDZERO);
  f = 1.3333;
  if (f != 0x1.555326p+0f)
    abort ();
#endif
  return 0;
}


> > 2021-10-27  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR middle-end/57245
> > 	* fold-const.c (fold_convert_const_real_from_real): Honor
> > 	-frounding-math if the conversion is not exact.
> > ---
> >  gcc/fold-const.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> > index ff23f12f33c..c7aebf9cc7e 100644
> > --- a/gcc/fold-const.c
> > +++ b/gcc/fold-const.c
> > @@ -2139,6 +2139,12 @@ fold_convert_const_real_from_real (tree type, const_tree arg1)
> >        && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
> >      return NULL_TREE; 
> >  
> > +  /* With flag_rounding_math we shuld respect the current rounding mode
> 
> s/shuld/should/
> 
> > +     unless the conversion is exact.  */
> > +  if (HONOR_SIGN_DEPENDENT_ROUNDING (arg1)
> > +      && !exact_real_truncate (TYPE_MODE (type), &TREE_REAL_CST (arg1)))
> > +    return NULL_TREE;
> > +
> >    real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
> >    t = build_real (type, value);
> 
> 	Jakub
> 
>
Jakub Jelinek Oct. 27, 2021, 2:44 p.m. UTC | #3
On Wed, Oct 27, 2021 at 04:29:38PM +0200, Richard Biener wrote:
> So something like the following below?  Note I have to fix 
> simplify_const_unary_operation to not perform the invalid constant
> folding with (not worrying about the exact conversion case - I doubt
> any of the constant folding is really relevant on RTL these days,
> maybe we should simply punt for all unary float-float ops when either
> mode has sign dependent rounding modes)
> 
> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> index bbbd6b74942..9522a31570e 100644
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
> @@ -2068,6 +2073,9 @@ simplify_const_unary_operation (enum rtx_code code, 
> machine_mode mode,
>              and the operand is a signaling NaN.  */
>           if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
>             return NULL_RTX;
> +         /* Or if flag_rounding_math is on.  */
> +         if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
> +           return NULL_RTX;
>           d = real_value_truncate (mode, d);
>           break;

Won't this stop folding of truncations that are never a problem?
I mean at least if the wider float mode constant is exactly representable
in the narrower float mode, no matter what rounding mode is used the value
will be always the same...
And people use
  float f = 1.0;
or
  float f = 1.25;
etc. a lot.
So perhaps again
	if (HONOR_SIGN_DEPENDENT_ROUNDING (mode)
	    && !exact_real_truncate (mode, &d))
	  return NULL_RTX;
?

> /* PR57245 */
> /* { dg-do run } */
> /* { dg-require-effective-target fenv } */
> /* { dg-additional-options "-frounding-math" } */
> 
> #include <fenv.h>
> #include <stdlib.h>
> 
> int
> main ()
> {

Roughly yes.  Some tests also do #ifdef FE_*, so in your case
> #if __DBL_MANT_DIG__ == 53 && __FLT_MANT_DIG__ == 24
+#ifdef FE_UPWARD
>   fesetround (FE_UPWARD);
>   float f = 1.3;
>   if (f != 0x1.4ccccep+0f)
>     __builtin_abort ();
+#endif
+#ifdef FE_TONEAREST
etc.
>   fesetround (FE_TONEAREST);
>   /* Use different actual values so the bogus CSE we perform does not
>      break things.  */
>   f = 1.33;
>   if (f != 0x1.547ae2p+0f)
>     abort ();
>   fesetround (FE_DOWNWARD);
>   f = 1.333;
>   if (f != 0x1.553f7cp+0f)
>     abort ();
>   fesetround (FE_TOWARDZERO);
>   f = 1.3333;
>   if (f != 0x1.555326p+0f)
>     abort ();
> #endif
>   return 0;
> }

	Jakub
Richard Biener Oct. 27, 2021, 3:52 p.m. UTC | #4
On October 27, 2021 4:44:53 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Wed, Oct 27, 2021 at 04:29:38PM +0200, Richard Biener wrote:
>> So something like the following below?  Note I have to fix 
>> simplify_const_unary_operation to not perform the invalid constant
>> folding with (not worrying about the exact conversion case - I doubt
>> any of the constant folding is really relevant on RTL these days,
>> maybe we should simply punt for all unary float-float ops when either
>> mode has sign dependent rounding modes)
>> 
>> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
>> index bbbd6b74942..9522a31570e 100644
>> --- a/gcc/simplify-rtx.c
>> +++ b/gcc/simplify-rtx.c
>> @@ -2068,6 +2073,9 @@ simplify_const_unary_operation (enum rtx_code code, 
>> machine_mode mode,
>>              and the operand is a signaling NaN.  */
>>           if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
>>             return NULL_RTX;
>> +         /* Or if flag_rounding_math is on.  */
>> +         if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
>> +           return NULL_RTX;
>>           d = real_value_truncate (mode, d);
>>           break;
>
>Won't this stop folding of truncations that are never a problem?
>I mean at least if the wider float mode constant is exactly representable
>in the narrower float mode, no matter what rounding mode is used the value
>will be always the same...
>And people use
>  float f = 1.0;
>or
>  float f = 1.25;
>etc. a lot.

Yes, but I do expect any such opportunities to be realized on GENERIC/GIMPLE? 

>So perhaps again
>	if (HONOR_SIGN_DEPENDENT_ROUNDING (mode)
>	    && !exact_real_truncate (mode, &d))
>	  return NULL_RTX;
>?

Sure, for this case it's short and straight forward. 

>
>> /* PR57245 */
>> /* { dg-do run } */
>> /* { dg-require-effective-target fenv } */
>> /* { dg-additional-options "-frounding-math" } */
>> 
>> #include <fenv.h>
>> #include <stdlib.h>
>> 
>> int
>> main ()
>> {
>
>Roughly yes.  Some tests also do #ifdef FE_*, so in your case
>> #if __DBL_MANT_DIG__ == 53 && __FLT_MANT_DIG__ == 24
>+#ifdef FE_UPWARD

Ah, OK. Will fix. 

Richard. 

>>   fesetround (FE_UPWARD);
>>   float f = 1.3;
>>   if (f != 0x1.4ccccep+0f)
>>     __builtin_abort ();
>+#endif
>+#ifdef FE_TONEAREST
>etc.
>>   fesetround (FE_TONEAREST);
>>   /* Use different actual values so the bogus CSE we perform does not
>>      break things.  */
>>   f = 1.33;
>>   if (f != 0x1.547ae2p+0f)
>>     abort ();
>>   fesetround (FE_DOWNWARD);
>>   f = 1.333;
>>   if (f != 0x1.553f7cp+0f)
>>     abort ();
>>   fesetround (FE_TOWARDZERO);
>>   f = 1.3333;
>>   if (f != 0x1.555326p+0f)
>>     abort ();
>> #endif
>>   return 0;
>> }
>
>	Jakub
>
Richard Biener Oct. 28, 2021, 8:11 a.m. UTC | #5
On Wed, 27 Oct 2021, Richard Biener wrote:

> On October 27, 2021 4:44:53 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
> >On Wed, Oct 27, 2021 at 04:29:38PM +0200, Richard Biener wrote:
> >> So something like the following below?  Note I have to fix 
> >> simplify_const_unary_operation to not perform the invalid constant
> >> folding with (not worrying about the exact conversion case - I doubt
> >> any of the constant folding is really relevant on RTL these days,
> >> maybe we should simply punt for all unary float-float ops when either
> >> mode has sign dependent rounding modes)
> >> 
> >> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> >> index bbbd6b74942..9522a31570e 100644
> >> --- a/gcc/simplify-rtx.c
> >> +++ b/gcc/simplify-rtx.c
> >> @@ -2068,6 +2073,9 @@ simplify_const_unary_operation (enum rtx_code code, 
> >> machine_mode mode,
> >>              and the operand is a signaling NaN.  */
> >>           if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
> >>             return NULL_RTX;
> >> +         /* Or if flag_rounding_math is on.  */
> >> +         if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
> >> +           return NULL_RTX;
> >>           d = real_value_truncate (mode, d);
> >>           break;
> >
> >Won't this stop folding of truncations that are never a problem?
> >I mean at least if the wider float mode constant is exactly representable
> >in the narrower float mode, no matter what rounding mode is used the value
> >will be always the same...
> >And people use
> >  float f = 1.0;
> >or
> >  float f = 1.25;
> >etc. a lot.
> 
> Yes, but I do expect any such opportunities to be realized on GENERIC/GIMPLE? 
> 
> >So perhaps again
> >	if (HONOR_SIGN_DEPENDENT_ROUNDING (mode)
> >	    && !exact_real_truncate (mode, &d))
> >	  return NULL_RTX;
> >?
> 
> Sure, for this case it's short and straight forward. 
> 
> >
> >> /* PR57245 */
> >> /* { dg-do run } */
> >> /* { dg-require-effective-target fenv } */
> >> /* { dg-additional-options "-frounding-math" } */
> >> 
> >> #include <fenv.h>
> >> #include <stdlib.h>
> >> 
> >> int
> >> main ()
> >> {
> >
> >Roughly yes.  Some tests also do #ifdef FE_*, so in your case
> >> #if __DBL_MANT_DIG__ == 53 && __FLT_MANT_DIG__ == 24
> >+#ifdef FE_UPWARD
> 
> Ah, OK. Will fix. 

So like this - bootstrapped and tested on x86_64-unknown-linux-gnu.

OK now?

Thanks,
Richard.

From 22da541c70ec2eff1e9208dd53c6d7309c33b0c9 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguenther@suse.de>
Date: Wed, 27 Oct 2021 14:27:40 +0200
Subject: [PATCH] middle-end/57245 - honor -frounding-math in real truncation
To: gcc-patches@gcc.gnu.org

The following honors -frounding-math when converting a FP constant
to another FP type.

2021-10-27  Richard Biener  <rguenther@suse.de>

	PR middle-end/57245
	* fold-const.c (fold_convert_const_real_from_real): Honor
	-frounding-math if the conversion is not exact.
	* simplify-rtx.c (simplify_const_unary_operation): Do not
	simplify FLOAT_TRUNCATE with sign dependent rounding.

	* gcc.dg/torture/fp-double-convert-float-1.c: New testcase.
---
 gcc/fold-const.c                              |  6 +++
 gcc/simplify-rtx.c                            |  5 +++
 .../torture/fp-double-convert-float-1.c       | 41 +++++++++++++++++++
 3 files changed, 52 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-double-convert-float-1.c

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index ff23f12f33c..18950aeb760 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -2139,6 +2139,12 @@ fold_convert_const_real_from_real (tree type, const_tree arg1)
       && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
     return NULL_TREE; 
 
+  /* With flag_rounding_math we should respect the current rounding mode
+     unless the conversion is exact.  */
+  if (HONOR_SIGN_DEPENDENT_ROUNDING (arg1)
+      && !exact_real_truncate (TYPE_MODE (type), &TREE_REAL_CST (arg1)))
+    return NULL_TREE;
+
   real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
   t = build_real (type, value);
 
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index bbbd6b74942..f38b6d7d31c 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -2068,6 +2068,11 @@ simplify_const_unary_operation (enum rtx_code code, machine_mode mode,
 	     and the operand is a signaling NaN.  */
 	  if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
 	    return NULL_RTX;
+	  /* Or if flag_rounding_math is on and the truncation is not
+	     exact.  */
+	  if (HONOR_SIGN_DEPENDENT_ROUNDING (mode)
+	      && !exact_real_truncate (mode, &d))
+	    return NULL_RTX;
 	  d = real_value_truncate (mode, d);
 	  break;
 	case FLOAT_EXTEND:
diff --git a/gcc/testsuite/gcc.dg/torture/fp-double-convert-float-1.c b/gcc/testsuite/gcc.dg/torture/fp-double-convert-float-1.c
new file mode 100644
index 00000000000..ec23274ea98
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-double-convert-float-1.c
@@ -0,0 +1,41 @@
+/* PR57245 */
+/* { dg-do run } */
+/* { dg-require-effective-target fenv } */
+/* { dg-additional-options "-frounding-math" } */
+
+#include <fenv.h>
+#include <stdlib.h>
+
+int
+main ()
+{
+#if __DBL_MANT_DIG__ == 53 && __FLT_MANT_DIG__ == 24
+#ifdef FE_UPWARD
+  fesetround (FE_UPWARD);
+  float f = 1.3;
+  if (f != 0x1.4ccccep+0f)
+    __builtin_abort ();
+#endif
+#ifdef FE_TONEAREST
+  fesetround (FE_TONEAREST);
+  /* Use different actual values so the bogus CSE we perform does not
+     break things.  */
+  f = 1.33;
+  if (f != 0x1.547ae2p+0f)
+    abort ();
+#endif
+#ifdef FE_DOWNWARD
+  fesetround (FE_DOWNWARD);
+  f = 1.333;
+  if (f != 0x1.553f7cp+0f)
+    abort ();
+#endif
+#ifdef FE_TOWARDZERO
+  fesetround (FE_TOWARDZERO);
+  f = 1.3333;
+  if (f != 0x1.555326p+0f)
+    abort ();
+#endif
+#endif
+  return 0;
+}
Jakub Jelinek Oct. 28, 2021, 9:22 a.m. UTC | #6
On Thu, Oct 28, 2021 at 10:11:36AM +0200, Richard Biener wrote:
> 2021-10-27  Richard Biener  <rguenther@suse.de>
> 
> 	PR middle-end/57245
> 	* fold-const.c (fold_convert_const_real_from_real): Honor
> 	-frounding-math if the conversion is not exact.
> 	* simplify-rtx.c (simplify_const_unary_operation): Do not
> 	simplify FLOAT_TRUNCATE with sign dependent rounding.
> 
> 	* gcc.dg/torture/fp-double-convert-float-1.c: New testcase.

LGTM, thanks.

	Jakub
H.J. Lu Dec. 17, 2021, 2:25 p.m. UTC | #7
On Thu, Oct 28, 2021 at 2:23 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Thu, Oct 28, 2021 at 10:11:36AM +0200, Richard Biener wrote:
> > 2021-10-27  Richard Biener  <rguenther@suse.de>
> >
> >       PR middle-end/57245
> >       * fold-const.c (fold_convert_const_real_from_real): Honor
> >       -frounding-math if the conversion is not exact.
> >       * simplify-rtx.c (simplify_const_unary_operation): Do not
> >       simplify FLOAT_TRUNCATE with sign dependent rounding.
> >
> >       * gcc.dg/torture/fp-double-convert-float-1.c: New testcase.
>
> LGTM, thanks.
>
>         Jakub
>

This miscompiled libm in glibc:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103735
diff mbox series

Patch

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index ff23f12f33c..c7aebf9cc7e 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -2139,6 +2139,12 @@  fold_convert_const_real_from_real (tree type, const_tree arg1)
       && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
     return NULL_TREE; 
 
+  /* With flag_rounding_math we shuld respect the current rounding mode
+     unless the conversion is exact.  */
+  if (HONOR_SIGN_DEPENDENT_ROUNDING (arg1)
+      && !exact_real_truncate (TYPE_MODE (type), &TREE_REAL_CST (arg1)))
+    return NULL_TREE;
+
   real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
   t = build_real (type, value);