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\]+" } } */

Reply via email to