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

["Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]


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

[-- Attachment #2: trunc.patch --]
[-- Type: text/plain, Size: 2024 bytes --]

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