[PING] Re: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64

  Hi,

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html

The patch is attached as text for ease of use. Is there anything that needs to change?

Ok for master? If OK, can it be committed for me, I have no commit rights.

Jirui Wu

-----Original Message-----
From: Jirui Wu 
Sent: Friday, September 10, 2021 10:14 AM
To: Richard Biener <rguenther@suse.de>
Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers <joseph@codesourcery.com>
Subject: [PING] Re: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64

Hi,

Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html

Ok for master? If OK, can it be committed for me, I have no commit rights.

Jirui Wu
-----Original Message-----
From: Jirui Wu
Sent: Friday, September 3, 2021 12:39 PM
To: 'Richard Biener' <rguenther@suse.de>
Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers <joseph@codesourcery.com>
Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64

Ping

-----Original Message-----
From: Jirui Wu
Sent: Friday, August 20, 2021 4:28 PM
To: Richard Biener <rguenther@suse.de>
Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers <joseph@codesourcery.com>
Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64

> -----Original Message-----
> From: Richard Biener <rguenther@suse.de>
> Sent: Friday, August 20, 2021 8:15 AM
> To: Jirui Wu <Jirui.Wu@arm.com>
> Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski 
> <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; 
> ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers 
> <joseph@codesourcery.com>
> Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for
> (double)(int) under -ffast-math on aarch64
> 
> On Thu, 19 Aug 2021, Jirui Wu wrote:
> 
> > Hi all,
> >
> > This patch generates FRINTZ instruction to optimize type casts.
> >
> > The changes in this patch covers:
> > * Generate FRINTZ for (double)(int) casts.
> > * Add new test cases.
> >
> > The intermediate type is not checked according to the C99 spec.
> > Overflow of the integral part when casting floats to integers causes
> undefined behavior.
> > As a result, optimization to trunc() is not invalid.
> > I've confirmed that Boolean type does not match the matching condition.
> >
> > Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master? If OK can it be committed for me, I have no commit rights.
> 
> +/* Detected a fix_trunc cast inside a float type cast,
> +   use IFN_TRUNC to optimize.  */
> +#if GIMPLE
> +(simplify
> +  (float (fix_trunc @0))
> +  (if (direct_internal_fn_supported_p (IFN_TRUNC, type,
> +                                      OPTIMIZE_FOR_BOTH)
> +       && flag_unsafe_math_optimizations
> +       && type == TREE_TYPE (@0))
> 
> types_match (type, TREE_TYPE (@0))
> 
> please.  Please perform cheap tests first (the flag test).
> 
> +     (IFN_TRUNC @0)))
> +#endif
> 
> why only for GIMPLE?  I'm not sure flag_unsafe_math_optimizations is a 
> good test here.  If you say we can use undefined behavior of any 
> overflow of the fix_trunc operation what do we guard here?
> If it's Inf/NaN input then flag_finite_math_only would be more 
> appropriate, if it's behavior for -0. (I suppose trunc (-0.0) == -0.0 
> and thus "wrong") then a && !HONOR_SIGNED_ZEROS (type) is missing 
> instead.  If it's setting of FENV state and possibly trapping on 
> overflow (but it's undefined?!) then flag_trapping_math covers the 
> latter but we don't have any flag for eliding FENV state affecting 
> transforms, so there the kitchen-sink flag_unsafe_math_optimizations might apply.
> 
> So - which is it?
> 
This change is only for GIMPLE because we can't test for the optab support without being in GIMPLE. direct_internal_fn_supported_p is defined only for GIMPLE. 

IFN_TRUNC's documentation mentions nothing for zero, NaNs/inf inputs.
So I think the correct guard is just flag_fp_int_builtin_inexact.
!flag_trapping_math because the operation can only still raise inexacts.

The new pattern is moved next to the place you mentioned.

Ok for master? If OK can it be committed for me, I have no commit rights.

Thanks,
Jirui
> Note there's also the pattern
> 
> /* Handle cases of two conversions in a row.  */ (for ocvt (convert 
> float
> fix_trunc)  (for icvt (convert float)
>   (simplify
>    (ocvt (icvt@1 @0))
>    (with
>     {
> ...
> 
> which is related so please put the new pattern next to that (the set 
> of conversions handled there does not include (float (fix_trunc @0)))
> 
> Thanks,
> Richard.
> 
> > Thanks,
> > Jirui
> >
> > gcc/ChangeLog:
> >
> >         * match.pd: Generate IFN_TRUNC.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/aarch64/merge_trunc1.c: New test.
> >
> > > -----Original Message-----
> > > From: Richard Biener <richard.guenther@gmail.com>
> > > Sent: Tuesday, August 17, 2021 9:13 AM
> > > To: Andrew Pinski <pinskia@gmail.com>
> > > Cc: Jirui Wu <Jirui.Wu@arm.com>; Richard Sandiford 
> > > <Richard.Sandiford@arm.com>; ian@airs.com; 
> > > gcc-patches@gcc.gnu.org; rguenther@suse.de
> > > Subject: Re: [Patch][GCC][middle-end] - Generate FRINTZ for
> > > (double)(int) under -ffast-math on aarch64
> > >
> > > On Mon, Aug 16, 2021 at 8:48 PM Andrew Pinski via Gcc-patches
> > > <gcc- patches@gcc.gnu.org> wrote:
> > > >
> > > > On Mon, Aug 16, 2021 at 9:15 AM Jirui Wu via Gcc-patches 
> > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > This patch generates FRINTZ instruction to optimize type casts.
> > > > >
> > > > > The changes in this patch covers:
> > > > > * Opimization of a FIX_TRUNC_EXPR cast inside a FLOAT_EXPR 
> > > > > using
> > > IFN_TRUNC.
> > > > > * Change of corresponding test cases.
> > > > >
> > > > > Regtested on aarch64-none-linux-gnu and no issues.
> > > > >
> > > > > Ok for master? If OK can it be committed for me, I have no 
> > > > > commit
> rights.
> > > >
> > > > Is there a reason why you are doing the transformation manually 
> > > > inside forwprop rather than handling it inside match.pd?
> > > > Also can't this only be done for -ffast-math case?
> > >
> > > You definitely have to look at the intermediate type - that could 
> > > be a uint8_t or even a boolean type.  So unless the intermediate 
> > > type can represent all float values optimizing to trunc() is invalid.
> > > Also if you emit IFN_TRUNC you have to make sure there's target 
> > > support - we don't emit calls to a library
> > > trunc() from an internal function call (and we wouldn't want to 
> > > optimize it that way).
> > >
> > > Richard.
> > >
> > > >
> > > > Thanks,
> > > > Andrew Pinski
> > > >
> > > > >
> > > > > Thanks,
> > > > > Jirui
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > >         * tree-ssa-forwprop.c (pass_forwprop::execute): 
> > > > > Optimize with
> frintz.
> > > > >
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > >         * gcc.target/aarch64/fix_trunc1.c: Update to new expectation.
> >
> 
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 
> Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
  

On Fri, Sep 24, 2021 at 2:59 PM Jirui Wu via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html
>
> The patch is attached as text for ease of use. Is there anything that needs to change?
>
> Ok for master? If OK, can it be committed for me, I have no commit rights.

I'm still not sure about the correctness.  I suppose the
flag_fp_int_builtin_inexact && !flag_trapping_math is supposed to guard
against spurious inexact exceptions, shouldn't that be
!flag_fp_int_builtin_inexact || !flag_trapping_math instead?

The comment looks a bit redundant and we prefer sth like

/* (double)(int)x -> trunc (x) if the type of x matches the
expressions FP type.  */

Thanks,
Richard.

> Jirui Wu
>
> -----Original Message-----
> From: Jirui Wu
> Sent: Friday, September 10, 2021 10:14 AM
> To: Richard Biener <rguenther@suse.de>
> Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers <joseph@codesourcery.com>
> Subject: [PING] Re: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64
>
> Hi,
>
> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html
>
> Ok for master? If OK, can it be committed for me, I have no commit rights.
>
> Jirui Wu
> -----Original Message-----
> From: Jirui Wu
> Sent: Friday, September 3, 2021 12:39 PM
> To: 'Richard Biener' <rguenther@suse.de>
> Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers <joseph@codesourcery.com>
> Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64
>
> Ping
>
> -----Original Message-----
> From: Jirui Wu
> Sent: Friday, August 20, 2021 4:28 PM
> To: Richard Biener <rguenther@suse.de>
> Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>; ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers <joseph@codesourcery.com>
> Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under -ffast-math on aarch64
>
> > -----Original Message-----
> > From: Richard Biener <rguenther@suse.de>
> > Sent: Friday, August 20, 2021 8:15 AM
> > To: Jirui Wu <Jirui.Wu@arm.com>
> > Cc: Richard Biener <richard.guenther@gmail.com>; Andrew Pinski
> > <pinskia@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>;
> > ian@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers
> > <joseph@codesourcery.com>
> > Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for
> > (double)(int) under -ffast-math on aarch64
> >
> > On Thu, 19 Aug 2021, Jirui Wu wrote:
> >
> > > Hi all,
> > >
> > > This patch generates FRINTZ instruction to optimize type casts.
> > >
> > > The changes in this patch covers:
> > > * Generate FRINTZ for (double)(int) casts.
> > > * Add new test cases.
> > >
> > > The intermediate type is not checked according to the C99 spec.
> > > Overflow of the integral part when casting floats to integers causes
> > undefined behavior.
> > > As a result, optimization to trunc() is not invalid.
> > > I've confirmed that Boolean type does not match the matching condition.
> > >
> > > Regtested on aarch64-none-linux-gnu and no issues.
> > >
> > > Ok for master? If OK can it be committed for me, I have no commit rights.
> >
> > +/* Detected a fix_trunc cast inside a float type cast,
> > +   use IFN_TRUNC to optimize.  */
> > +#if GIMPLE
> > +(simplify
> > +  (float (fix_trunc @0))
> > +  (if (direct_internal_fn_supported_p (IFN_TRUNC, type,
> > +                                      OPTIMIZE_FOR_BOTH)
> > +       && flag_unsafe_math_optimizations
> > +       && type == TREE_TYPE (@0))
> >
> > types_match (type, TREE_TYPE (@0))
> >
> > please.  Please perform cheap tests first (the flag test).
> >
> > +     (IFN_TRUNC @0)))
> > +#endif
> >
> > why only for GIMPLE?  I'm not sure flag_unsafe_math_optimizations is a
> > good test here.  If you say we can use undefined behavior of any
> > overflow of the fix_trunc operation what do we guard here?
> > If it's Inf/NaN input then flag_finite_math_only would be more
> > appropriate, if it's behavior for -0. (I suppose trunc (-0.0) == -0.0
> > and thus "wrong") then a && !HONOR_SIGNED_ZEROS (type) is missing
> > instead.  If it's setting of FENV state and possibly trapping on
> > overflow (but it's undefined?!) then flag_trapping_math covers the
> > latter but we don't have any flag for eliding FENV state affecting
> > transforms, so there the kitchen-sink flag_unsafe_math_optimizations might apply.
> >
> > So - which is it?
> >
> This change is only for GIMPLE because we can't test for the optab support without being in GIMPLE. direct_internal_fn_supported_p is defined only for GIMPLE.
>
> IFN_TRUNC's documentation mentions nothing for zero, NaNs/inf inputs.
> So I think the correct guard is just flag_fp_int_builtin_inexact.
> !flag_trapping_math because the operation can only still raise inexacts.
>
> The new pattern is moved next to the place you mentioned.
>
> Ok for master? If OK can it be committed for me, I have no commit rights.
>
> Thanks,
> Jirui
> > Note there's also the pattern
> >
> > /* Handle cases of two conversions in a row.  */ (for ocvt (convert
> > float
> > fix_trunc)  (for icvt (convert float)
> >   (simplify
> >    (ocvt (icvt@1 @0))
> >    (with
> >     {
> > ...
> >
> > which is related so please put the new pattern next to that (the set
> > of conversions handled there does not include (float (fix_trunc @0)))
> >
> > Thanks,
> > Richard.
> >
> > > Thanks,
> > > Jirui
> > >
> > > gcc/ChangeLog:
> > >
> > >         * match.pd: Generate IFN_TRUNC.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/aarch64/merge_trunc1.c: New test.
> > >
> > > > -----Original Message-----
> > > > From: Richard Biener <richard.guenther@gmail.com>
> > > > Sent: Tuesday, August 17, 2021 9:13 AM
> > > > To: Andrew Pinski <pinskia@gmail.com>
> > > > Cc: Jirui Wu <Jirui.Wu@arm.com>; Richard Sandiford
> > > > <Richard.Sandiford@arm.com>; ian@airs.com;
> > > > gcc-patches@gcc.gnu.org; rguenther@suse.de
> > > > Subject: Re: [Patch][GCC][middle-end] - Generate FRINTZ for
> > > > (double)(int) under -ffast-math on aarch64
> > > >
> > > > On Mon, Aug 16, 2021 at 8:48 PM Andrew Pinski via Gcc-patches
> > > > <gcc- patches@gcc.gnu.org> wrote:
> > > > >
> > > > > On Mon, Aug 16, 2021 at 9:15 AM Jirui Wu via Gcc-patches
> > > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > This patch generates FRINTZ instruction to optimize type casts.
> > > > > >
> > > > > > The changes in this patch covers:
> > > > > > * Opimization of a FIX_TRUNC_EXPR cast inside a FLOAT_EXPR
> > > > > > using
> > > > IFN_TRUNC.
> > > > > > * Change of corresponding test cases.
> > > > > >
> > > > > > Regtested on aarch64-none-linux-gnu and no issues.
> > > > > >
> > > > > > Ok for master? If OK can it be committed for me, I have no
> > > > > > commit
> > rights.
> > > > >
> > > > > Is there a reason why you are doing the transformation manually
> > > > > inside forwprop rather than handling it inside match.pd?
> > > > > Also can't this only be done for -ffast-math case?
> > > >
> > > > You definitely have to look at the intermediate type - that could
> > > > be a uint8_t or even a boolean type.  So unless the intermediate
> > > > type can represent all float values optimizing to trunc() is invalid.
> > > > Also if you emit IFN_TRUNC you have to make sure there's target
> > > > support - we don't emit calls to a library
> > > > trunc() from an internal function call (and we wouldn't want to
> > > > optimize it that way).
> > > >
> > > > Richard.
> > > >
> > > > >
> > > > > Thanks,
> > > > > Andrew Pinski
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Jirui
> > > > > >
> > > > > > gcc/ChangeLog:
> > > > > >
> > > > > >         * tree-ssa-forwprop.c (pass_forwprop::execute):
> > > > > > Optimize with
> > frintz.
> > > > > >
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > >
> > > > > >         * gcc.target/aarch64/fix_trunc1.c: Update to new expectation.
> > >
> >
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
  

On Fri, 15 Oct 2021, Richard Biener via Gcc-patches wrote:

> On Fri, Sep 24, 2021 at 2:59 PM Jirui Wu via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi,
> >
> > Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html
> >
> > The patch is attached as text for ease of use. Is there anything that needs to change?
> >
> > Ok for master? If OK, can it be committed for me, I have no commit rights.
> 
> I'm still not sure about the correctness.  I suppose the
> flag_fp_int_builtin_inexact && !flag_trapping_math is supposed to guard
> against spurious inexact exceptions, shouldn't that be
> !flag_fp_int_builtin_inexact || !flag_trapping_math instead?

The following remarks may be relevant here, but are not intended as an 
assertion of what is correct in this case.

1. flag_fp_int_builtin_inexact is the more permissive case ("inexact" may 
or may not be raised).  All existing uses in back ends are 
"flag_fp_int_builtin_inexact || !flag_trapping_math" or equivalent.

2. flag_fp_int_builtin_inexact only applies to certain built-in functions 
(as listed in invoke.texi).  It's always unspecified, even in C2X, whether 
casts of non-integer values from floating-point to integer types raise 
"inexact".  So flag_fp_int_builtin_inexact should not be checked in insn 
patterns corresponding to simple casts from floating-point to integer, 
only in insn patterns corresponding to the built-in functions listed for 
-fno-fp-int-builtin-inexact in invoke.texi (or for operations that combine 
such a built-in function with a cast of the *result* to integer type).
  

On 19/10/2021 00:22, Joseph Myers wrote:
> On Fri, 15 Oct 2021, Richard Biener via Gcc-patches wrote:
>
>> On Fri, Sep 24, 2021 at 2:59 PM Jirui Wu via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>> Hi,
>>>
>>> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html
>>>
>>> The patch is attached as text for ease of use. Is there anything that needs to change?
>>>
>>> Ok for master? If OK, can it be committed for me, I have no commit rights.
>> I'm still not sure about the correctness.  I suppose the
>> flag_fp_int_builtin_inexact && !flag_trapping_math is supposed to guard
>> against spurious inexact exceptions, shouldn't that be
>> !flag_fp_int_builtin_inexact || !flag_trapping_math instead?
> The following remarks may be relevant here, but are not intended as an
> assertion of what is correct in this case.
>
> 1. flag_fp_int_builtin_inexact is the more permissive case ("inexact" may
> or may not be raised).  All existing uses in back ends are
> "flag_fp_int_builtin_inexact || !flag_trapping_math" or equivalent.
>
> 2. flag_fp_int_builtin_inexact only applies to certain built-in functions
> (as listed in invoke.texi).  It's always unspecified, even in C2X, whether
> casts of non-integer values from floating-point to integer types raise
> "inexact".  So flag_fp_int_builtin_inexact should not be checked in insn
> patterns corresponding to simple casts from floating-point to integer,
> only in insn patterns corresponding to the built-in functions listed for
> -fno-fp-int-builtin-inexact in invoke.texi (or for operations that combine
> such a built-in function with a cast of the *result* to integer type).
Hi,

I agree with Joseph, I don't think we should be checking 
flag_fp_int_builtin_inexact here because we aren't transforming the math 
function 'trunc', but rather a piece of C-code that has trunc-like 
semantics.

As for flag_trapping_math, it's definition says 'Assume floating point 
operations can trap'. I assume IFN_TRUNC would not trap, since I don't 
think IFN_TRUNC will preserve the overflow behaviour, in the cases where 
the FP value is bigger than the intermediate integer type range. So I 
think we should prevent the transformation if we are assuming the FP 
instructions can trap.

If we don't assume the FP instructions can trap, then I think it's fine 
to ignore the overflow as this behavior is undefined in C.

Also changed the comment. Slightly different to your suggestion Richard, 
in an attempt to be more generic. Do you still have concerns regarding 
the checks?

Kind regards,
Andre
diff --git a/gcc/match.pd b/gcc/match.pd
index 3ff15bc0de5aba45ade94ca6e47e01fad9a2a314..5bed2e12715ea213813ef8b84fd420475b04d201 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3606,6 +3606,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 >= inside_prec - !inside_unsignedp)
      (convert @0)))))))
 
+/* (float_type)(integer_type) x -> trunc (x) if the type of x matches
+   float_type.  Only do the transformation if we do not need to preserve
+   trapping behaviour, so require !flag_trapping_math. */
+#if GIMPLE
+(simplify
+   (float (fix_trunc @0))
+   (if (!flag_trapping_math
+	&& types_match (type, TREE_TYPE (@0))
+	&& direct_internal_fn_supported_p (IFN_TRUNC, type,
+					  OPTIMIZE_FOR_BOTH))
+      (IFN_TRUNC @0)))
+#endif
+
 /* If we have a narrowing conversion to an integral type that is fed by a
    BIT_AND_EXPR, we might be able to remove the BIT_AND_EXPR if it merely
    masks off bits outside the final type (and nothing else).  */
diff --git a/gcc/testsuite/gcc.target/aarch64/merge_trunc1.c b/gcc/testsuite/gcc.target/aarch64/merge_trunc1.c
new file mode 100644
index 0000000000000000000000000000000000000000..07217064e2ba54fcf4f5edc440e6ec19ddae66e1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/merge_trunc1.c
@@ -0,0 +1,41 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ffast-math" } */
+
+float
+f1 (float x)
+{
+  int y = x;
+
+  return (float) y;
+}
+
+double
+f2 (double x)
+{
+  long y = x;
+
+  return (double) y;
+}
+
+float
+f3 (double x)
+{
+  int y = x;
+
+  return (float) y;
+}
+
+double
+f4 (float x)
+{
+  int y = x;
+
+  return (double) y;
+}
+
+/* { dg-final { scan-assembler "frintz\\ts\[0-9\]+, s\[0-9\]+" } } */
+/* { dg-final { scan-assembler "frintz\\td\[0-9\]+, d\[0-9\]+" } } */
+/* { dg-final { scan-assembler "fcvtzs\\tw\[0-9\]+, d\[0-9\]+" } } */
+/* { dg-final { scan-assembler "scvtf\\ts\[0-9\]+, w\[0-9\]+" } } */
+/* { dg-final { scan-assembler "fcvtzs\\tw\[0-9\]+, s\[0-9\]+" } } */
+/* { dg-final { scan-assembler "scvtf\\td\[0-9\]+, w\[0-9\]+" } } */
  

On Wed, 20 Oct 2021, Andre Vieira (lists) wrote:

> 
> On 19/10/2021 00:22, Joseph Myers wrote:
> > On Fri, 15 Oct 2021, Richard Biener via Gcc-patches wrote:
> >
> >> On Fri, Sep 24, 2021 at 2:59 PM Jirui Wu via Gcc-patches
> >> <gcc-patches@gcc.gnu.org> wrote:
> >>> Hi,
> >>>
> >>> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html
> >>>
> >>> The patch is attached as text for ease of use. Is there anything that
> >>> needs to change?
> >>>
> >>> Ok for master? If OK, can it be committed for me, I have no commit rights.
> >> I'm still not sure about the correctness.  I suppose the
> >> flag_fp_int_builtin_inexact && !flag_trapping_math is supposed to guard
> >> against spurious inexact exceptions, shouldn't that be
> >> !flag_fp_int_builtin_inexact || !flag_trapping_math instead?
> > The following remarks may be relevant here, but are not intended as an
> > assertion of what is correct in this case.
> >
> > 1. flag_fp_int_builtin_inexact is the more permissive case ("inexact" may
> > or may not be raised).  All existing uses in back ends are
> > "flag_fp_int_builtin_inexact || !flag_trapping_math" or equivalent.
> >
> > 2. flag_fp_int_builtin_inexact only applies to certain built-in functions
> > (as listed in invoke.texi).  It's always unspecified, even in C2X, whether
> > casts of non-integer values from floating-point to integer types raise
> > "inexact".  So flag_fp_int_builtin_inexact should not be checked in insn
> > patterns corresponding to simple casts from floating-point to integer,
> > only in insn patterns corresponding to the built-in functions listed for
> > -fno-fp-int-builtin-inexact in invoke.texi (or for operations that combine
> > such a built-in function with a cast of the *result* to integer type).
> Hi,
> 
> I agree with Joseph, I don't think we should be checking
> flag_fp_int_builtin_inexact here because we aren't transforming the math
> function 'trunc', but rather a piece of C-code that has trunc-like semantics.

But we are generating 'trunc' which may now raise a spurious exception.
OTOH flag_fp_int_builtin_inexact wouldn't help here because
"may or may not" stil may raise spurious exception flags.

> As for flag_trapping_math, it's definition says 'Assume floating point
> operations can trap'. I assume IFN_TRUNC would not trap, since I don't think
> IFN_TRUNC will preserve the overflow behaviour, in the cases where the FP
> value is bigger than the intermediate integer type range. So I think we should
> prevent the transformation if we are assuming the FP instructions can trap.

Note trap == set exception flags, not only raise a trap.

> If we don't assume the FP instructions can trap, then I think it's fine to
> ignore the overflow as this behavior is undefined in C.
> 
> Also changed the comment. Slightly different to your suggestion Richard, in an
> attempt to be more generic. Do you still have concerns regarding the checks?

I think your updated patch is OK.

Thanks,
Richard.