On Fri, Apr 24, 2020 at 5:05 AM Zhanghaijian (A)
<z.zhanghaij...@huawei.com> wrote:
>
> Thanks for your suggestions. For safety, I put back
> flag_unsafe_math_optimizations.
> Attached please find the v3 patch which is suitable for git am. Bootstrap and
> tested on aarch64 Linux platform.
> Is the v3 patch ok?
OK.
Thanks,
Richard.
> From aafcad7ed5d3d156eee1cabbb4958e2cce7899c6 Mon Sep 17 00:00:00 2001
> From: z00219097 <z.zhanghaij...@huawei.com>
> Date: Fri, 24 Apr 2020 08:56:25 +0800
> Subject: [PATCH] rtl combine should consider NaNs when generate fp min/max
> [PR94708]
>
> As discussed on PR94708, it's unsafe for rtl combine to generate fp
> min/max under -funsafe-math-optimizations, considering NaNs. In
> addition to flag_unsafe_math_optimizations check, we also need to
> do extra mode feature testing here: && !HONOR_NANS (mode)
> && !HONOR_SIGNED_ZEROS (mode)
>
> 2020-04-24 Haijian Zhang <z.zhanghaij...@huawei.com>
>
> gcc/
> PR rtl-optimization/94708
> * combine.c (simplify_if_then_else): Add check for
> !HONOR_NANS (mode) && !HONOR_SIGNED_ZEROS (mode).
> gcc/testsuite/
> PR fortran/94708
> * gfortran.dg/pr94708.f90: New test.
> ---
> gcc/ChangeLog | 6 ++++++
> gcc/combine.c | 5 ++++-
> gcc/testsuite/ChangeLog | 5 +++++
> gcc/testsuite/gfortran.dg/pr94708.f90 | 13 +++++++++++++
> 4 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gfortran.dg/pr94708.f90
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index cf97cfed626..6032e681d7f 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2020-04-24 Haijian Zhang <z.zhanghaij...@huawei.com>
> +
> + PR rtl-optimization/94708
> + * combine.c (simplify_if_then_else): Add check for
> + !HONOR_NANS (mode) && !HONOR_SIGNED_ZEROS (mode).
> +
> 2020-04-23 Martin Sebor <mse...@redhat.com>
>
> PR driver/90983
> diff --git a/gcc/combine.c b/gcc/combine.c
> index cff76cd3303..4c324f38660 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -6643,7 +6643,10 @@ simplify_if_then_else (rtx x)
>
> /* Look for MIN or MAX. */
>
> - if ((! FLOAT_MODE_P (mode) || flag_unsafe_math_optimizations)
> + if ((! FLOAT_MODE_P (mode)
> + || (flag_unsafe_math_optimizations
> + && !HONOR_NANS (mode)
> + && !HONOR_SIGNED_ZEROS (mode)))
> && comparison_p
> && rtx_equal_p (XEXP (cond, 0), true_rtx)
> && rtx_equal_p (XEXP (cond, 1), false_rtx)
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 86331cd1211..4d80ee3a3a0 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-04-24 Haijian Zhang <z.zhanghaij...@huawei.com>
> +
> + PR fortran/94708
> + * gfortran.dg/pr94708.f90: New test.
> +
> 2020-04-23 Martin Sebor <mse...@redhat.com>
>
> PR driver/90983
> diff --git a/gcc/testsuite/gfortran.dg/pr94708.f90
> b/gcc/testsuite/gfortran.dg/pr94708.f90
> new file mode 100644
> index 00000000000..9b5f1389f09
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr94708.f90
> @@ -0,0 +1,13 @@
> +! { dg-do compile { target aarch64*-*-* } }
> +! { dg-options "-O2 -funsafe-math-optimizations -fdump-rtl-combine" }
> +
> +subroutine f(vara,varb,varc,res)
> + REAL, INTENT(IN) :: vara,varb,varc
> + REAL, INTENT(out) :: res
> +
> + res = vara
> + if (res .lt. varb) res = varb
> + if (res .gt. varc) res = varc
> +end subroutine
> +
> +! { dg-final { scan-rtl-dump-not "smin" "combine" } }
> --
> 2.19.1
>
>
> -----Original Message-----
> From: Segher Boessenkool [mailto:seg...@kernel.crashing.org]
> Sent: Friday, April 24, 2020 1:05 AM
> To: Zhanghaijian (A) <z.zhanghaij...@huawei.com>
> Cc: Richard Biener <richard.guent...@gmail.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR94708] rtl combine should consider NaNs when generate
> fp min/max
>
> Hi!
>
> On Thu, Apr 23, 2020 at 02:34:03PM +0000, Zhanghaijian (A) wrote:
> > Thanks for your suggestions. I have modified accordingly.
> > Attached please find the adapted patch. Bootstrap and tested on aarch64
> > Linux platform.
> > Does the v2 patch look better?
> >
> > diff --git a/gcc/combine.c b/gcc/combine.c index
> > cff76cd3303..ad8a385fc48 100644
> > --- a/gcc/combine.c
> > +++ b/gcc/combine.c
> > @@ -6643,7 +6643,8 @@ simplify_if_then_else (rtx x)
> >
> > /* Look for MIN or MAX. */
> >
> > - if ((! FLOAT_MODE_P (mode) || flag_unsafe_math_optimizations)
> > + if ((! FLOAT_MODE_P (mode)
> > + || (!HONOR_NANS (mode) && !HONOR_SIGNED_ZEROS (mode)))
> > && comparison_p
> > && rtx_equal_p (XEXP (cond, 0), true_rtx)
> > && rtx_equal_p (XEXP (cond, 1), false_rtx)
>
> > > The GENERIC folding routine producing min/max is avoiding it when
> > > those are honored (and it doesn't check
> > > flag_unsafe_math_optmizations at all).
> > >
> > > Certainly the patch is an incremental correct fix, with the flag
> > > testing replaced by the mode feature testing.
> >
> > Yeah, and the SMAX etc. definition is so weak that it isn't obvious
> > that this combine transform is valid without this flag. We can or
> > should fix that, of course :-)
>
> Please put flag_unsafe_math_optimizations back? It isn't clear at all that
> we do not need it.
>
> Also, do you have a changelog entry?
>
>
> Segher
