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 <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: [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' <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)
