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?
>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
pr94708-v3.patch
Description: pr94708-v3.patch
