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' <rguent...@suse.de>
Cc: Richard Biener <richard.guent...@gmail.com>; Andrew Pinski 
<pins...@gmail.com>; Richard Sandiford <richard.sandif...@arm.com>; 
i...@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers 
<jos...@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 <rguent...@suse.de>
Cc: Richard Biener <richard.guent...@gmail.com>; Andrew Pinski 
<pins...@gmail.com>; Richard Sandiford <richard.sandif...@arm.com>; 
i...@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers 
<jos...@codesourcery.com>
Subject: RE: [Patch][GCC][middle-end] - Generate FRINTZ for (double)(int) under 
-ffast-math on aarch64

> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Friday, August 20, 2021 8:15 AM
> To: Jirui Wu <jirui...@arm.com>
> Cc: Richard Biener <richard.guent...@gmail.com>; Andrew Pinski 
> <pins...@gmail.com>; Richard Sandiford <richard.sandif...@arm.com>; 
> i...@airs.com; gcc-patches@gcc.gnu.org; Joseph S. Myers 
> <jos...@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.guent...@gmail.com>
> > > Sent: Tuesday, August 17, 2021 9:13 AM
> > > To: Andrew Pinski <pins...@gmail.com>
> > > Cc: Jirui Wu <jirui...@arm.com>; Richard Sandiford 
> > > <richard.sandif...@arm.com>; i...@airs.com; 
> > > gcc-patches@gcc.gnu.org; rguent...@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- patc...@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 <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 
> Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Attachment: diff
Description: diff

Reply via email to