Hi Jim:

This patch is fixing the wrong behavior for unordered float compare
for Signaling NaN, current implementation will suppress the FP
exception flags unconditionally, however signaling NaN should signal
an exception according IEEE 754-2008 spec.

I've add an test for that, and full test is included in glibc/math,
glibc/math testsuite report is attached.

Tested with RV32GC and RV64GC linux gcc and glibc testsuite, no new
failed case introduced.

Attachment: subdir-tests.sum
Description: Binary data

From 320ceae90c1d70e2254ee84ea30d89ed32bfb9da Mon Sep 17 00:00:00 2001
From: Monk Chiang <m...@andestech.com>
Date: Fri, 14 Sep 2018 16:25:45 +0800
Subject: [PATCH] RISC-V: Fix unordered float compare for signaling NaN.

 - Old implementation will suppress the FP exception flags unconditionally,
   however signaling NaN should signal an exception according IEEE 754-2008
   spec.

ChangeLog:

2018-10-03  Monk Chiang <m...@andestech.com>
	    Kito Cheng <k...@andestech.com>

gcc/
	* config/riscv/riscv.md (f<quiet_pattern>_quiet<ANYF:mode><X:mode>4):
	Handle signaling NaN correctly.
	testsuite/gcc.target/riscv/fcompare_snan.c: New file.
---
 gcc/config/riscv/riscv.md                     | 41 ++++++++++++++++-
 .../gcc.target/riscv/fcompare_snan.c          | 45 +++++++++++++++++++
 2 files changed, 84 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/fcompare_snan.c

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 4162dc578e8..8dd64a83c01 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1965,10 +1965,47 @@
 	     QUIET_COMPARISON))
     (clobber (match_scratch:X 3 "=&r"))]
   "TARGET_HARD_FLOAT"
-  "frflags\t%3\n\tf<quiet_pattern>.<fmt>\t%0,%1,%2\n\tfsflags %3"
+{
+  /* This pattern is consist of 3 parts:
+     1. Check source opreands has any NaN.
+        feq tmp1, src1, src1
+        feq tmp2, src2, src2
+     2. Ignore comparison if any source operands is NaN.
+        and dst, tmp1, tmp2
+        beqz dst, 1f
+
+     3. Do the comparison.
+        f[lt|le] dst, src1, src2
+
+     1. Check inputs has any NaN:
+
+     Using FEQ instructions to check the operands is NaN by compare those self.
+
+     2. Ignore comparison if any NaN.
+
+     Jump to ahead of actual comparison instruction if one of source operand is
+     NaN value, because it's satisfied the semantics of the original
+     comparisons, destination register is set to 0 in step 1 if any source
+     operands are NaN (both qNaN and sNaN).
+
+     According RISC-V User Level ISA spec 8.8, "FEQ.S performs a
+     quiet comparison: only signaling NaN inputs cause an Invalid Operation
+     exception.", it set the right exception flags in step 1.
+
+     3. Do the comparison.
+
+     In this point, both operands are not NaN, so just do the comparison.
+   */
+  return "feq.<fmt>\t%0, %1, %1\n\t"
+	 "feq.<fmt>\t%3, %2, %2\n\t"
+	 "and\t%0, %0, %3\n\t"
+	 "beqz\t%0, 1f\n\t"
+	 "f<quiet_pattern>.<fmt>\t%0,%1,%2\n\t"
+         "1:";
+}
   [(set_attr "type" "fcmp")
    (set_attr "mode" "<UNITMODE>")
-   (set (attr "length") (const_int 12))])
+   (set (attr "length") (const_int 20))])
 
 (define_insn "*seq_zero_<X:mode><GPR:mode>"
   [(set (match_operand:GPR       0 "register_operand" "=r")
diff --git a/gcc/testsuite/gcc.target/riscv/fcompare_snan.c b/gcc/testsuite/gcc.target/riscv/fcompare_snan.c
new file mode 100644
index 00000000000..9dd059ddd08
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/fcompare_snan.c
@@ -0,0 +1,45 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fenv_exceptions } */
+/* { dg-options "-O" } */
+
+#include <math.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <fenv.h>
+
+volatile double minus_zero = -0.0;
+
+int test(double op0, double op1, int except) __attribute__((noinline));
+int test(double op0, double op1, int except)
+{
+  int ret;
+  feclearexcept (FE_ALL_EXCEPT);
+
+  ret = __builtin_isgreater (op0, op1);
+
+  if (fetestexcept(FE_ALL_EXCEPT) != except)
+    abort ();
+
+  return ret;
+}
+
+int main()
+{
+  int ret, fflags = 0;
+  volatile double qnan = __builtin_nan("");
+  volatile double snan = __builtin_nans("");
+
+  test (minus_zero, snan, FE_INVALID);
+  test (snan, minus_zero, FE_INVALID);
+  test (minus_zero, qnan, 0);
+  test (qnan, minus_zero, 0);
+  test (1.0, snan, FE_INVALID);
+  test (snan, 1.0, FE_INVALID);
+  test (1.0, qnan, 0);
+  test (qnan, 1.0, 0);
+  test (qnan, snan, FE_INVALID);
+  test (snan, qnan, FE_INVALID);
+
+  return 0;
+}
-- 
2.17.1

Reply via email to