On Mon, 11 May 2020, Joseph Myers wrote:

> On Fri, 8 May 2020, Richard Biener wrote:
> 
> > The IFNs are supposed to match fmin and fmax from the C standard which 
> > IIRC have IEEE semantics.
> 
> fmin and fmax have IEEE (2008) semantics (where an sNaN operand results in 
> a qNaN result with "invalid" raised", but a quiet NaN results in the other 
> operand, if not sNaN, being returned).  Not to be confused with any of the 
> new minimum/maximum operations in IEEE (2019) (both variants that treat 
> all NaNs like other arithmetic operations, and variants that can raise 
> "invalid" for sNaN without returning a NaN), for which C bindings under 
> different names are proposed.

Uh, ok - that makes the situation even more messy in the future
(exact rules how to combine maximumX (fmin (X, Y), X) or so...).

In the mean time I'm considering the following for my immediate
needs which are PR88540 not basic-block vectorizing FP min/max w/o 
-ffast-math and testcase breakage in gcc.target/i386/pr54855-[89].c
where conditional move handling on RTL no longer happens when
GIMPLE starts to be more clever.

The patch below makes phiopt if-convert min/max-like operations
for floating-point types regardless of HONOR_NANS/HONOR_SIGNED_ZEROS
because targets likely implement instructions for C conditional
operator-style min/max-like operations.

Now x86 does so for d1[n] < d2[n] ? d1[n] : d2[n] but not
d1[n] <= d2[n] ? d1[n] : d2[n] which is a subtle difference but
the patch below still if-converts those.

I've chosen not to try to combine with existing min/max operations
(nor try detecting the COND_EXPR form as such) but instead give up.
I could have given up for <= and >= operations as well (and not
if-convert), if there's reasons to believe no targets implement
single instructions for those.

Note I specifically do not want general if-conversion to
COND_EXPRs applied here (so that's a bit of conflicting interest).
But there's no standard pattern names for those conditional
min/max-like operations (and I'm not sure we need them).

Thanks,
Richard.

[PATCH] tree-optimization/88540 - apply phiopt to min/max-like operation

This applies if conversion in phiopt to a < b ? a : b like computations
which are not MIN or MAX_EXPR or fmin or fmax due to NaNs and
signed zeros.  Instead of giving up this makes phiopt emit
a COND_EXPR.  This helps vectorization of straight-line code and
avoids messing up the pattern by PRE.

2020-05-12  Richard Biener  <rguent...@suse.de>

        PR tree-optimization/88540
        * tree-ssa-phiopt.c (minmax_replacement): When honoring
        NaNs or signed zeros instead emit a COND_EXPR.

        * gcc.target/i386/pr88540-1.c: New testcase.
---
 gcc/testsuite/gcc.target/i386/pr88540-1.c | 21 +++++++++++++++++++++
 gcc/tree-ssa-phiopt.c                     | 25 ++++++++++++++++++-------
 2 files changed, 39 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr88540-1.c

diff --git a/gcc/testsuite/gcc.target/i386/pr88540-1.c 
b/gcc/testsuite/gcc.target/i386/pr88540-1.c
new file mode 100644
index 00000000000..a66b89ef66c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr88540-1.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-vectorize -msse2" } */
+
+void test1(double* __restrict d1, double* __restrict d2, double* __restrict d3)
+{
+  for (int n = 0; n < 2; ++n)
+    {
+      d3[n] = d1[n] < d2[n] ? d1[n] : d2[n];
+    }
+}
+
+void test2(double* __restrict d1, double* __restrict d2, double* __restrict d3)
+{
+  for (int n = 0; n < 2; ++n)
+    {
+      d3[n] = d1[n] > d2[n] ? d1[n] : d2[n];
+    }
+}
+
+/* { dg-final { scan-assembler "minpd" } } */
+/* { dg-final { scan-assembler "maxpd" } } */
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index b1e0dce93d8..eac8dcdd696 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -1369,13 +1369,10 @@ minmax_replacement (basic_block cond_bb, basic_block 
middle_bb,
   enum tree_code cmp, minmax, ass_code;
   tree smaller, alt_smaller, larger, alt_larger, arg_true, arg_false;
   gimple_stmt_iterator gsi, gsi_from;
+  bool ieee_fp;
 
   type = TREE_TYPE (PHI_RESULT (phi));
-
-  /* The optimization may be unsafe due to NaNs.  */
-  if (HONOR_NANS (type) || HONOR_SIGNED_ZEROS (type))
-    return false;
-
+  ieee_fp = (HONOR_NANS (type) || HONOR_SIGNED_ZEROS (type));
   cond = as_a <gcond *> (last_stmt (cond_bb));
   cmp = gimple_cond_code (cond);
   rhs = gimple_cond_rhs (cond);
@@ -1527,6 +1524,10 @@ minmax_replacement (basic_block cond_bb, basic_block 
middle_bb,
         b = MAX (a, d);
         x = MIN (b, u);  */
 
+      /* But not when we have to care bout NaNs or singed zeros.  */
+      if (ieee_fp)
+       return false;
+
       gimple *assign = last_and_only_stmt (middle_bb);
       tree lhs, op0, op1, bound;
 
@@ -1697,8 +1698,18 @@ minmax_replacement (basic_block cond_bb, basic_block 
middle_bb,
   else
     result = make_ssa_name (TREE_TYPE (result));
 
-  /* Emit the statement to compute min/max.  */
-  new_stmt = gimple_build_assign (result, minmax, arg0, arg1);
+  /* Emit the statement to compute min/max.  If the optimization is
+     unsafe due to NaNs or signed zeros preserve the not commutative
+     operation as a COND_EXPR.  */
+  if (ieee_fp)
+    new_stmt = gimple_build_assign (result, COND_EXPR,
+                                   build2 (gimple_cond_code (cond),
+                                           boolean_type_node,
+                                           gimple_cond_lhs (cond),
+                                           gimple_cond_rhs (cond)),
+                                   arg_true, arg_false);
+  else
+    new_stmt = gimple_build_assign (result, minmax, arg0, arg1);
   gsi = gsi_last_bb (cond_bb);
   gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT);
 
-- 
2.13.7

Reply via email to