Re: [PATCH] Skip constant folding for fmin/max when either argument is sNaN [PR105414]

[Richard Biener <richard.guenther@gmail.com>]

On Thu, May 5, 2022 at 10:30 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> on 2022/5/5 16:09, Richard Biener via Gcc-patches wrote:
> > On Thu, May 5, 2022 at 10:07 AM HAO CHEN GUI via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> Hi,
> >>    This patch skips constant folding for fmin/max when either argument
> >> is sNaN. According to C standard,
> >>    fmin(sNaN, sNaN）= qNaN, fmin(sNaN, NaN) = qNaN
> >>    So signaling NaN should be tested and skipped for fmin/max in match.pd.
> >>
> >>    Bootstrapped and tested on ppc64 Linux BE and LE with no regressions.
> >> Is this okay for trunk? Any recommendations? Thanks a lot.
> >
> > OK.
> >
> > Thanks,
> > Richard.
> >
> >> ChangeLog
> >>
> >> 2022-05-05 Haochen Gui <guihaoc@linux.ibm.com>
> >>
> >> gcc/
> >>         PR target/105414
> >>         * match.pd (minmax): Skip constant folding for fmin/fmax when both
> >>         arguments are sNaN or one is sNaN and another is NaN.
> >>
> >> gcc/testsuite/
> >>         PR target/105414
> >>         * gcc.dg/pr105414.c: New.
> >>
> >> patch.diff
> >>
> >> diff --git a/gcc/match.pd b/gcc/match.pd
> >> index cad61848daa..f256bcbb483 100644
> >> --- a/gcc/match.pd
> >> +++ b/gcc/match.pd
> >> @@ -3093,7 +3093,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >>  (for minmax (min max FMIN_ALL FMAX_ALL)
> >>   (simplify
> >>    (minmax @0 @0)
> >> -  @0))
> >> +  /* if both are sNaN, it should return qNaN.  */
> >> +  (if (!tree_expr_maybe_signaling_nan_p (@0))
> >> +    @0)))
>
> Sorry for chiming in.
>
> IIUC this patch is mainly for libc function fmin/fmax and the iterator here
> covers min/max and fmin/fmax.  I wonder if it's intent to make this change
> for min/max as well?
>
> As tree.def, "if either operand is NaN, then it is unspecified", the optimization
> for min/max seems still acceptable?

MIN/MAX_EXPR shouldn't even appear with -fsignalling-nans for this
reason, at least that's what I thought.  But yes, you might have a point
here (but maybe it's also not strictly enough specified).  One option would
be to do (minmax == MAX_EXPR || minmax == MIN_EXPR || !tree_expr ...)

Joseph - are MIN_EXPR and MAX_EXPR supposed to turn sNaN into qNaN
and the 'undefinedness' is merely as to which operand is chosen?

Richard.

>
> BR,
> Kewen