middle-end/84407 - honor -frounding-math for int to float conversion

  This makes us honor -frounding-math for integer to float conversions
and avoid constant folding when such conversion is not exact.

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

Thanks,
Richard.

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

	PR middle-end/84407
	* fold-const.c (fold_convert_const): Avoid int to float
	constant folding with -frounding-math and inexact result.
	* simplify-rtx.c (simplify_const_unary_operation): Likewise.

	* gcc.dg/torture/fp-uint64-convert-double-1.c: New testcase.
---
 gcc/fold-const.c                              | 15 +++-
 gcc/simplify-rtx.c                            | 13 ++++
 .../torture/fp-uint64-convert-double-1.c      | 74 +++++++++++++++++++
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-1.c
  

On Thu, Oct 28, 2021 at 01:32:17PM +0200, Richard Biener wrote:
> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> index f38b6d7d31c..a16395befcd 100644
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
> @@ -1917,6 +1917,19 @@ simplify_const_unary_operation (enum rtx_code code, machine_mode mode,
>          return 0;
>  
>        d = real_value_truncate (mode, d);
> +
> +      /* Avoid the folding if flag_rounding_math is on and the
> +	 conversion is not exact.  */
> +      if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
> +	{
> +	  bool fail = false;
> +	  wide_int w = real_to_integer (&d, &fail,
> +					GET_MODE_PRECISION
> +					  (as_a <scalar_int_mode> (op_mode)));
> +	  if (fail || wi::ne_p (w, wide_int (rtx_mode_t (op, op_mode))))
> +	    return 0;
> +	}
> +
>        return const_double_from_real_value (d, mode);
>      }
>    else if (code == UNSIGNED_FLOAT && CONST_SCALAR_INT_P (op))

What about the else if case (i.e. UNSIGNED_FLOAT)?
And I think it would be nice to test the simplify-rtx.c code somewhere,
perhaps gcc/testsuite/gcc.dg/rtl/x86_64 testcase and check that we
simplify with -frounding-math e.g. UNSIGNED_FLOAT from DImode
0x8000000000000000 or FLOAT or UNSIGNED_FLOAT from DImode
0x7ffffffffffffc00, but will not fold FLOAT or UNSIGNED_FLOAT from
DImode 0x7ffffffffffffc01 or 0x7fffffffffffffff.

	Jakub
  

On Thu, 28 Oct 2021, Jakub Jelinek wrote:

> On Thu, Oct 28, 2021 at 01:32:17PM +0200, Richard Biener wrote:
> > diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> > index f38b6d7d31c..a16395befcd 100644
> > --- a/gcc/simplify-rtx.c
> > +++ b/gcc/simplify-rtx.c
> > @@ -1917,6 +1917,19 @@ simplify_const_unary_operation (enum rtx_code code, machine_mode mode,
> >          return 0;
> >  
> >        d = real_value_truncate (mode, d);
> > +
> > +      /* Avoid the folding if flag_rounding_math is on and the
> > +	 conversion is not exact.  */
> > +      if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
> > +	{
> > +	  bool fail = false;
> > +	  wide_int w = real_to_integer (&d, &fail,
> > +					GET_MODE_PRECISION
> > +					  (as_a <scalar_int_mode> (op_mode)));
> > +	  if (fail || wi::ne_p (w, wide_int (rtx_mode_t (op, op_mode))))
> > +	    return 0;
> > +	}
> > +
> >        return const_double_from_real_value (d, mode);
> >      }
> >    else if (code == UNSIGNED_FLOAT && CONST_SCALAR_INT_P (op))
> 
> What about the else if case (i.e. UNSIGNED_FLOAT)?

I'm not able to trigger unsigned_float to be used, even when
converting 0x8000000000000001 I get (float:DF (reg:DI...))
on x86_64 because we emit conditional code that will end up
using some compensation to emulate unsigned_float with
float with some tricks that do not necessarily look safe
from a rounding perspective (so maybe x86 would need to
resort to soft-fp here?):

        movabsq $4611686018427387905, %rax
        cvtsi2sdq       %rax, %xmm0
        addsd   %xmm0, %xmm0
        ucomisd .LC0(%rip), %xmm0

the constant is (0x8000000000000001u >> 1) | 1

> And I think it would be nice to test the simplify-rtx.c code somewhere,
> perhaps gcc/testsuite/gcc.dg/rtl/x86_64 testcase and check that we
> simplify with -frounding-math e.g. UNSIGNED_FLOAT from DImode
> 0x8000000000000000 or FLOAT or UNSIGNED_FLOAT from DImode
> 0x7ffffffffffffc00, but will not fold FLOAT or UNSIGNED_FLOAT from
> DImode 0x7ffffffffffffc01 or 0x7fffffffffffffff.

That it is not folded is exercised by the testcase already.  I indeed
have no good way to test actual folding besides an RTL testcase
(and I didn't add one for GIMPLE).

But as said elsehwere I don't see the RTL constant folding code
as important with regard to floats, but of course we have to fix it up.

The patch as-is fixes the reported testcase on x86_64, a target
eventually not implementing float but only unsigned_float might be
still broken.  I can put the same code in the unsigned_float code
but I have no way of exercising it.

Anyway, it feels like I spent too much time on this already for what
was supposed to be low-hanging fruit ;)

Richard.
  

On Thu, Oct 28, 2021 at 02:24:23PM +0200, Richard Biener wrote:
> I'm not able to trigger unsigned_float to be used, even when
> converting 0x8000000000000001 I get (float:DF (reg:DI...))
> on x86_64 because we emit conditional code that will end up
> using some compensation to emulate unsigned_float with
> float with some tricks that do not necessarily look safe
> from a rounding perspective (so maybe x86 would need to
> resort to soft-fp here?):
> 
>         movabsq $4611686018427387905, %rax
>         cvtsi2sdq       %rax, %xmm0
>         addsd   %xmm0, %xmm0
>         ucomisd .LC0(%rip), %xmm0
> 
> the constant is (0x8000000000000001u >> 1) | 1

Missing -mavx512f ?
(define_expand "floatunsdidf2"
  [(set (match_operand:DF 0 "register_operand")
        (unsigned_float:DF
          (match_operand:DI 1 "nonimmediate_operand")))]
  "((TARGET_64BIT && TARGET_AVX512F)
    || TARGET_KEEPS_VECTOR_ALIGNED_STACK)
   && TARGET_SSE2 && TARGET_SSE_MATH"
{
  if (!TARGET_64BIT)
    {
      ix86_expand_convert_uns_didf_sse (operands[0], operands[1]);
      DONE;
    }
  if (!TARGET_AVX512F)
    {
      x86_emit_floatuns (operands);
      DONE;
    }
})
where x86_emit_floatuns emits that emulation?
Anyway, what the testcase probably needs to do is this
  (set (reg:DI temp1) (const_int ...))
  (set (reg:DF temp2) (unsigned_float:DF (reg:DI temp1))) ! And also float separately too
  (set (reg:DI temp3) (subreg:DF (reg:DF temp2)))
or something similar so that during combine it is not rejected because
it is not valid to have the DFmode constants as immediates and they'd need
to go into memory instead.  But the subreg might not be valid too.
So perhaps some different target.
Yet another option would be a self-test...

But if you don't have time for the testcase right now, let's just
handle it in UNSIGNED_FLOAT too and I can try to look at the testcase
later?

	Jakub
  

On Thu, 28 Oct 2021, Richard Biener wrote:

> On Thu, 28 Oct 2021, Jakub Jelinek wrote:
> 
> > On Thu, Oct 28, 2021 at 01:32:17PM +0200, Richard Biener wrote:
> > > diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> > > index f38b6d7d31c..a16395befcd 100644
> > > --- a/gcc/simplify-rtx.c
> > > +++ b/gcc/simplify-rtx.c
> > > @@ -1917,6 +1917,19 @@ simplify_const_unary_operation (enum rtx_code code, machine_mode mode,
> > >          return 0;
> > >  
> > >        d = real_value_truncate (mode, d);
> > > +
> > > +      /* Avoid the folding if flag_rounding_math is on and the
> > > +	 conversion is not exact.  */
> > > +      if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
> > > +	{
> > > +	  bool fail = false;
> > > +	  wide_int w = real_to_integer (&d, &fail,
> > > +					GET_MODE_PRECISION
> > > +					  (as_a <scalar_int_mode> (op_mode)));
> > > +	  if (fail || wi::ne_p (w, wide_int (rtx_mode_t (op, op_mode))))
> > > +	    return 0;
> > > +	}
> > > +
> > >        return const_double_from_real_value (d, mode);
> > >      }
> > >    else if (code == UNSIGNED_FLOAT && CONST_SCALAR_INT_P (op))
> > 
> > What about the else if case (i.e. UNSIGNED_FLOAT)?
> 
> I'm not able to trigger unsigned_float to be used, even when
> converting 0x8000000000000001 I get (float:DF (reg:DI...))
> on x86_64 because we emit conditional code that will end up
> using some compensation to emulate unsigned_float with
> float with some tricks that do not necessarily look safe
> from a rounding perspective (so maybe x86 would need to
> resort to soft-fp here?):
> 
>         movabsq $4611686018427387905, %rax
>         cvtsi2sdq       %rax, %xmm0
>         addsd   %xmm0, %xmm0
>         ucomisd .LC0(%rip), %xmm0
> 
> the constant is (0x8000000000000001u >> 1) | 1
> 
> > And I think it would be nice to test the simplify-rtx.c code somewhere,
> > perhaps gcc/testsuite/gcc.dg/rtl/x86_64 testcase and check that we
> > simplify with -frounding-math e.g. UNSIGNED_FLOAT from DImode
> > 0x8000000000000000 or FLOAT or UNSIGNED_FLOAT from DImode
> > 0x7ffffffffffffc00, but will not fold FLOAT or UNSIGNED_FLOAT from
> > DImode 0x7ffffffffffffc01 or 0x7fffffffffffffff.
> 
> That it is not folded is exercised by the testcase already.  I indeed
> have no good way to test actual folding besides an RTL testcase
> (and I didn't add one for GIMPLE).
> 
> But as said elsehwere I don't see the RTL constant folding code
> as important with regard to floats, but of course we have to fix it up.
> 
> The patch as-is fixes the reported testcase on x86_64, a target
> eventually not implementing float but only unsigned_float might be
> still broken.  I can put the same code in the unsigned_float code
> but I have no way of exercising it.
> 
> Anyway, it feels like I spent too much time on this already for what
> was supposed to be low-hanging fruit ;)

The following nevertheless adds a testcase with large enough
constants that might trigger unsigned_float plus a hunk to fix that.

Would that be OK?

Thanks,
Richard.

From bb57eaf45329e1dd0ccb0fe82b30e189d1cd86a4 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguenther@suse.de>
Date: Thu, 28 Oct 2021 11:38:32 +0200
Subject: [PATCH] middle-end/84407 - honor -frounding-math for int to float
 conversion
To: gcc-patches@gcc.gnu.org

This makes us honor -frounding-math for integer to float conversions
and avoid constant folding when such conversion is not exact.

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

	PR middle-end/84407
	* fold-const.c (fold_convert_const): Avoid int to float
	constant folding with -frounding-math and inexact result.
	* simplify-rtx.c (simplify_const_unary_operation): Likewise
	for both float and unsigned_float.

	* gcc.dg/torture/fp-uint64-convert-double-1.c: New testcase.
	* gcc.dg/torture/fp-uint64-convert-double-2.c: Likewise.
---
 gcc/fold-const.c                              | 15 +++-
 gcc/simplify-rtx.c                            | 26 +++++++
 .../torture/fp-uint64-convert-double-1.c      | 74 ++++++++++++++++++
 .../torture/fp-uint64-convert-double-2.c      | 75 +++++++++++++++++++
 4 files changed, 189 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-1.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-2.c

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 18950aeb760..c7daf871125 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -2290,7 +2290,20 @@ fold_convert_const (enum tree_code code, tree type, tree arg1)
   else if (TREE_CODE (type) == REAL_TYPE)
     {
       if (TREE_CODE (arg1) == INTEGER_CST)
-	return build_real_from_int_cst (type, arg1);
+	{
+	  tree res = build_real_from_int_cst (type, arg1);
+	  /* Avoid the folding if flag_rounding_math is on and the
+	     conversion is not exact.  */
+	  if (HONOR_SIGN_DEPENDENT_ROUNDING (type))
+	    {
+	      bool fail = false;
+	      wide_int w = real_to_integer (&TREE_REAL_CST (res), &fail,
+					    TYPE_PRECISION (TREE_TYPE (arg1)));
+	      if (fail || wi::ne_p (w, wi::to_wide (arg1)))
+		return NULL_TREE;
+	    }
+	  return res;
+	}
       else if (TREE_CODE (arg1) == REAL_CST)
 	return fold_convert_const_real_from_real (type, arg1);
       else if (TREE_CODE (arg1) == FIXED_CST)
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index f38b6d7d31c..a060f1bbce0 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1917,6 +1917,19 @@ simplify_const_unary_operation (enum rtx_code code, machine_mode mode,
         return 0;
 
       d = real_value_truncate (mode, d);
+
+      /* Avoid the folding if flag_rounding_math is on and the
+	 conversion is not exact.  */
+      if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
+	{
+	  bool fail = false;
+	  wide_int w = real_to_integer (&d, &fail,
+					GET_MODE_PRECISION
+					  (as_a <scalar_int_mode> (op_mode)));
+	  if (fail || wi::ne_p (w, wide_int (rtx_mode_t (op, op_mode))))
+	    return 0;
+	}
+
       return const_double_from_real_value (d, mode);
     }
   else if (code == UNSIGNED_FLOAT && CONST_SCALAR_INT_P (op))
@@ -1941,6 +1954,19 @@ simplify_const_unary_operation (enum rtx_code code, machine_mode mode,
         return 0;
 
       d = real_value_truncate (mode, d);
+
+      /* Avoid the folding if flag_rounding_math is on and the
+	 conversion is not exact.  */
+      if (HONOR_SIGN_DEPENDENT_ROUNDING (mode))
+	{
+	  bool fail = false;
+	  wide_int w = real_to_integer (&d, &fail,
+					GET_MODE_PRECISION
+					  (as_a <scalar_int_mode> (op_mode)));
+	  if (fail || wi::ne_p (w, wide_int (rtx_mode_t (op, op_mode))))
+	    return 0;
+	}
+
       return const_double_from_real_value (d, mode);
     }
 
diff --git a/gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-1.c b/gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-1.c
new file mode 100644
index 00000000000..b40a16a2257
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-1.c
@@ -0,0 +1,74 @@
+/* PR84407 */
+/* { dg-do run } */
+/* { dg-require-effective-target fenv } */
+/* { dg-additional-options "-frounding-math" } */
+
+#include <fenv.h>
+#include <stdlib.h>
+
+void __attribute__((noipa))
+fooa ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_TONEAREST
+  fesetround(FE_TONEAREST);
+  __UINT64_TYPE__ x = 0x7fffffffffffffff;
+  double f = x;
+  if (f != 0x1p+63)
+    abort ();
+#endif
+#endif
+}
+
+void __attribute__((noipa))
+foob ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_DOWNWARD
+  fesetround(FE_DOWNWARD);
+  __UINT64_TYPE__ x = 0x7fffffffffffffff;
+  double f = x;
+  if (f != 0x1.fffffffffffffp+62)
+    abort ();
+#endif
+#endif
+}
+
+void __attribute__((noipa))
+fooc ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_UPWARD
+  fesetround(FE_UPWARD);
+  __UINT64_TYPE__ x = 0x7fffffffffffffff;
+  double f = x;
+  if (f != 0x1p+63)
+    abort ();
+#endif
+#endif
+}
+
+void __attribute__((noipa))
+food ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_TOWARDZERO
+  fesetround(FE_TOWARDZERO);
+  __UINT64_TYPE__ x = 0x7fffffffffffffff;
+  double f = x;
+  if (f != 0x1.fffffffffffffp+62)
+    abort ();
+#endif
+#endif
+}
+
+
+int
+main ()
+{
+  fooa ();
+  foob ();
+  fooc ();
+  food ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-2.c b/gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-2.c
new file mode 100644
index 00000000000..952f96b33c9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-uint64-convert-double-2.c
@@ -0,0 +1,75 @@
+/* PR84407 */
+/* { dg-do run } */
+/* { dg-require-effective-target fenv } */
+/* { dg-additional-options "-frounding-math" } */
+
+#include <fenv.h>
+#include <stdlib.h>
+
+void __attribute__((noipa))
+fooa ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_TONEAREST
+  fesetround(FE_TONEAREST);
+  /* Large enough constant to trigger unsigned_float.  */
+  __UINT64_TYPE__ x = 0x8000000000000001;
+  double f = x;
+  if (f != 0x1p+63)
+    abort ();
+#endif
+#endif
+}
+
+void __attribute__((noipa))
+foob ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_DOWNWARD
+  fesetround(FE_DOWNWARD);
+  __UINT64_TYPE__ x = 0x8000000000000001;
+  double f = x;
+  if (f != 0x1p+63)
+    abort ();
+#endif
+#endif
+}
+
+void __attribute__((noipa))
+fooc ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_UPWARD
+  fesetround(FE_UPWARD);
+  __UINT64_TYPE__ x = 0x8000000000000001;
+  double f = x;
+  if (f != 0x1.0000000000001p+63)
+    abort ();
+#endif
+#endif
+}
+
+void __attribute__((noipa))
+food ()
+{
+#if __DBL_MANT_DIG__ == 53
+#ifdef FE_TOWARDZERO
+  fesetround(FE_TOWARDZERO);
+  __UINT64_TYPE__ x = 0x8000000000000001;
+  double f = x;
+  if (f != 0x1p+63)
+    abort ();
+#endif
+#endif
+}
+
+
+int
+main ()
+{
+  fooa ();
+  foob ();
+  fooc ();
+  food ();
+  return 0;
+}
  

On Thu, 28 Oct 2021, Jakub Jelinek wrote:

> On Thu, Oct 28, 2021 at 02:24:23PM +0200, Richard Biener wrote:
> > I'm not able to trigger unsigned_float to be used, even when
> > converting 0x8000000000000001 I get (float:DF (reg:DI...))
> > on x86_64 because we emit conditional code that will end up
> > using some compensation to emulate unsigned_float with
> > float with some tricks that do not necessarily look safe
> > from a rounding perspective (so maybe x86 would need to
> > resort to soft-fp here?):
> > 
> >         movabsq $4611686018427387905, %rax
> >         cvtsi2sdq       %rax, %xmm0
> >         addsd   %xmm0, %xmm0
> >         ucomisd .LC0(%rip), %xmm0
> > 
> > the constant is (0x8000000000000001u >> 1) | 1
> 
> Missing -mavx512f ?

Yeah, but I have no way to test AVX512 (well, I might try SDE but not
sure whether that handles rounding modes ;))  OK, so just trying
with that the -2 testcase indeed FAILs without the unsigned_float
hunk but succeeds with.  It also oddly succeeds with or without
the patch and the floatunsdidf emulation.

> (define_expand "floatunsdidf2"
>   [(set (match_operand:DF 0 "register_operand")
>         (unsigned_float:DF
>           (match_operand:DI 1 "nonimmediate_operand")))]
>   "((TARGET_64BIT && TARGET_AVX512F)
>     || TARGET_KEEPS_VECTOR_ALIGNED_STACK)
>    && TARGET_SSE2 && TARGET_SSE_MATH"
> {
>   if (!TARGET_64BIT)
>     {
>       ix86_expand_convert_uns_didf_sse (operands[0], operands[1]);
>       DONE;
>     }
>   if (!TARGET_AVX512F)
>     {
>       x86_emit_floatuns (operands);
>       DONE;
>     }
> })
> where x86_emit_floatuns emits that emulation?
> Anyway, what the testcase probably needs to do is this
>   (set (reg:DI temp1) (const_int ...))
>   (set (reg:DF temp2) (unsigned_float:DF (reg:DI temp1))) ! And also float separately too
>   (set (reg:DI temp3) (subreg:DF (reg:DF temp2)))
> or something similar so that during combine it is not rejected because
> it is not valid to have the DFmode constants as immediates and they'd need
> to go into memory instead.  But the subreg might not be valid too.
> So perhaps some different target.
> Yet another option would be a self-test...
> 
> But if you don't have time for the testcase right now, let's just
> handle it in UNSIGNED_FLOAT too and I can try to look at the testcase
> later?

Sure, that works for me.  See the patch I posted which is now in 
re-testing.

Thanks,
Richard.
  

On Thu, Oct 28, 2021 at 02:40:24PM +0200, Richard Biener wrote:
> This makes us honor -frounding-math for integer to float conversions
> and avoid constant folding when such conversion is not exact.
> 
> 2021-10-28  Richard Biener  <rguenther@suse.de>
> 
> 	PR middle-end/84407
> 	* fold-const.c (fold_convert_const): Avoid int to float
> 	constant folding with -frounding-math and inexact result.
> 	* simplify-rtx.c (simplify_const_unary_operation): Likewise
> 	for both float and unsigned_float.
> 
> 	* gcc.dg/torture/fp-uint64-convert-double-1.c: New testcase.
> 	* gcc.dg/torture/fp-uint64-convert-double-2.c: Likewise.

Ok, thanks.

	Jakub