diff mbox

, Fix PR 71869, Correctly implement isgreater, etc. on PowerPC __float128

Message ID 20160727000432.GA26496@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner July 27, 2016, 12:04 a.m. UTC
On Tue, Jul 26, 2016 at 06:26:19PM -0500, Segher Boessenkool wrote:
> On Tue, Jul 26, 2016 at 04:09:01PM -0400, Michael Meissner wrote:
> > > Could you test all five functions please?  Use multiple testcases, maybe.
> > 
> > I decided to write an executable test rather than do more assembly tests.  The
> > patch to rs6000.c is unchanged, and the test now tests all of the comparison
> > operators and functions with various values including quiet NaNs and signalling
> > NaNs.  It passes on power7 big endian 64-bit, power7 big endian 32-bit, power8
> > little endian 64-bit, and I ran it on the power9 simulator with hardware
> > float128 support.
> 
> > --- gcc/testsuite/gcc.target/powerpc/float128-cmp.c	(revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/float128-cmp.c	(revision 0)
> > @@ -0,0 +1,106 @@
> > +/* { dg-do compile { target { powerpc*-*-linux* } } } */
> > +/* { dg-require-effective-target ppc_float128_sw } */
> > +/* { dg-options "-mvsx -O2 -mfloat128" } */
> 
> dg-do compile?  That's not testing much then, as an executable test!

Good catch.  Hopefully, third time is a charm.  I verified that changing the
dg-do compile to dg-do run did run properly on a big endian power7 (both 32-bit
and 64-bit) and a little endian power8.

[gcc]
2016-07-26  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/71869
	* config/rs6000/rs6000.c (rs6000_generate_compare): Rework
	__float128 support when we don't have hardware support, so that
	the IEEE built-in functions like isgreater, first call __unordkf3
	to make sure neither operand is a NaN, and if both operands are
	ordered, do the normal comparison.

[gcc/testsuite]
2016-07-26  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/71869
	* gcc.target/powerpc/float128-cmp.c: New test to make sure that
	IEEE built-in functions handle quiet and signalling NaNs
	correctly.

Comments

Segher Boessenkool July 27, 2016, 12:58 a.m. UTC | #1
On Tue, Jul 26, 2016 at 08:04:32PM -0400, Michael Meissner wrote:
> > dg-do compile?  That's not testing much then, as an executable test!
> 
> Good catch.  Hopefully, third time is a charm.  I verified that changing the
> dg-do compile to dg-do run did run properly on a big endian power7 (both 32-bit
> and 64-bit) and a little endian power8.
> 
> [gcc]
> 2016-07-26  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR target/71869
> 	* config/rs6000/rs6000.c (rs6000_generate_compare): Rework
> 	__float128 support when we don't have hardware support, so that
> 	the IEEE built-in functions like isgreater, first call __unordkf3
> 	to make sure neither operand is a NaN, and if both operands are
> 	ordered, do the normal comparison.
> 
> [gcc/testsuite]
> 2016-07-26  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR target/71869
> 	* gcc.target/powerpc/float128-cmp.c: New test to make sure that
> 	IEEE built-in functions handle quiet and signalling NaNs
> 	correctly.

Okay for trunk.  Okay for 6 once there is independent testing (on
gcc-testresults, say) for all affected targets and you aren't comfortable
waiting any longer ;-)


Segher
Richard Biener July 27, 2016, 7:30 a.m. UTC | #2
On Tue, 26 Jul 2016, Segher Boessenkool wrote:

> On Tue, Jul 26, 2016 at 08:04:32PM -0400, Michael Meissner wrote:
> > > dg-do compile?  That's not testing much then, as an executable test!
> > 
> > Good catch.  Hopefully, third time is a charm.  I verified that changing the
> > dg-do compile to dg-do run did run properly on a big endian power7 (both 32-bit
> > and 64-bit) and a little endian power8.
> > 
> > [gcc]
> > 2016-07-26  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	PR target/71869
> > 	* config/rs6000/rs6000.c (rs6000_generate_compare): Rework
> > 	__float128 support when we don't have hardware support, so that
> > 	the IEEE built-in functions like isgreater, first call __unordkf3
> > 	to make sure neither operand is a NaN, and if both operands are
> > 	ordered, do the normal comparison.
> > 
> > [gcc/testsuite]
> > 2016-07-26  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	PR target/71869
> > 	* gcc.target/powerpc/float128-cmp.c: New test to make sure that
> > 	IEEE built-in functions handle quiet and signalling NaNs
> > 	correctly.
> 
> Okay for trunk.  Okay for 6 once there is independent testing (on
> gcc-testresults, say) for all affected targets and you aren't comfortable
> waiting any longer ;-)

You have at least two more weeks until GCC 6.2 RC1.

Richard.
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 238730)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -21755,8 +21755,8 @@  rs6000_generate_compare (rtx cmp, machin
   else if (!TARGET_FLOAT128_HW && FLOAT128_VECTOR_P (mode))
     {
       rtx libfunc = NULL_RTX;
-      bool uneq_or_ltgt = false;
-      rtx dest = gen_reg_rtx (SImode);
+      bool check_nan = false;
+      rtx dest;
 
       switch (code)
 	{
@@ -21783,21 +21783,23 @@  rs6000_generate_compare (rtx cmp, machin
 
 	case UNGE:
 	case UNGT:
-	  libfunc = optab_libfunc (le_optab, mode);
+	  check_nan = true;
+	  libfunc = optab_libfunc (ge_optab, mode);
 	  code = (code == UNGE) ? GE : GT;
 	  break;
 
 	case UNLE:
 	case UNLT:
-	  libfunc = optab_libfunc (ge_optab, mode);
+	  check_nan = true;
+	  libfunc = optab_libfunc (le_optab, mode);
 	  code = (code == UNLE) ? LE : LT;
 	  break;
 
 	case UNEQ:
 	case LTGT:
-	  libfunc = optab_libfunc (le_optab, mode);
-	  uneq_or_ltgt = true;
-	  code = (code = UNEQ) ? NE : EQ;
+	  check_nan = true;
+	  libfunc = optab_libfunc (eq_optab, mode);
+	  code = (code = UNEQ) ? EQ : NE;
 	  break;
 
 	default:
@@ -21805,21 +21807,56 @@  rs6000_generate_compare (rtx cmp, machin
 	}
 
       gcc_assert (libfunc);
-      dest = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST,
-				      SImode, 2, op0, mode, op1, mode);
 
-      /* If this is UNEQ or LTGT, we call __lekf2, which returns -1 for less
-	 than, 0 for equal, +1 for greater, and +2 for nan.  We add 1, to give
-	 a value of 0..3, and then do and AND immediate of 1 to isolate whether
-	 it is 0/Nan (i.e. bottom bit is 0), or less than/greater than
-	 (i.e. bottom bit is 1).  */
-      if (uneq_or_ltgt)
-	{
-	  rtx add_result = gen_reg_rtx (SImode);
-	  rtx and_result = gen_reg_rtx (SImode);
-	  emit_insn (gen_addsi3 (add_result, dest, GEN_INT (1)));
-	  emit_insn (gen_andsi3 (and_result, add_result, GEN_INT (1)));
-	  dest = and_result;
+      if (!check_nan)
+	dest = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST,
+					SImode, 2, op0, mode, op1, mode);
+
+      /* The library signals an exception for signalling NaNs, so we need to
+	 handle isgreater, etc. by first checking isordered.  */
+      else
+	{
+	  rtx ne_rtx, normal_dest, unord_dest;
+	  rtx unord_func = optab_libfunc (unord_optab, mode);
+	  rtx join_label = gen_label_rtx ();
+	  rtx join_ref = gen_rtx_LABEL_REF (VOIDmode, join_label);
+	  rtx unord_cmp = gen_reg_rtx (comp_mode);
+
+
+	  /* Test for either value being a NaN.  */
+	  gcc_assert (unord_func);
+	  unord_dest = emit_library_call_value (unord_func, NULL_RTX, LCT_CONST,
+						SImode, 2, op0, mode, op1,
+						mode);
+
+	  /* Set value (0) if either value is a NaN, and jump to the join
+	     label.  */
+	  dest = gen_reg_rtx (SImode);
+	  emit_move_insn (dest, const1_rtx);
+	  emit_insn (gen_rtx_SET (unord_cmp,
+				  gen_rtx_COMPARE (comp_mode, unord_dest,
+						   const0_rtx)));
+
+	  ne_rtx = gen_rtx_NE (comp_mode, unord_cmp, const0_rtx);
+	  emit_jump_insn (gen_rtx_SET (pc_rtx,
+				       gen_rtx_IF_THEN_ELSE (VOIDmode, ne_rtx,
+							     join_ref,
+							     pc_rtx)));
+
+	  /* Do the normal comparison, knowing that the values are not
+	     NaNs.  */
+	  normal_dest = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST,
+						 SImode, 2, op0, mode, op1,
+						 mode);
+
+	  emit_insn (gen_cstoresi4 (dest,
+				    gen_rtx_fmt_ee (code, SImode, normal_dest,
+						    const0_rtx),
+				    normal_dest, const0_rtx));
+
+	  /* Join NaN and non-Nan paths.  Compare dest against 0.  */
+	  emit_label (join_label);
+	  code = NE;
 	}
 
       emit_insn (gen_rtx_SET (compare_result,
Index: gcc/testsuite/gcc.target/powerpc/float128-cmp.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/float128-cmp.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/float128-cmp.c	(revision 0)
@@ -0,0 +1,106 @@ 
+/* { dg-do run { target { powerpc*-*-linux* } } } */
+/* { dg-require-effective-target ppc_float128_sw } */
+/* { dg-options "-mvsx -O2 -mfloat128" } */
+
+#include <stddef.h>
+#include <stdlib.h>
+
+#ifndef TYPE
+#define TYPE __float128
+#define NAN __builtin_nanq ("")
+#define SNAN __builtin_nansq ("")
+#else
+#define NAN __builtin_nan ("")
+#define SNAN __builtin_nans ("")
+#endif
+
+extern void check (TYPE a,
+		   TYPE b,
+		   int eq,
+		   int ne,
+		   int lt,
+		   int le,
+		   int gt,
+		   int ge,
+		   int i_lt,
+		   int i_le,
+		   int i_gt,
+		   int i_ge,
+		   int i_lg,
+		   int i_un) __attribute__((__noinline__));
+
+void
+check (TYPE a,
+       TYPE b,
+       int eq,
+       int ne,
+       int lt,
+       int le,
+       int gt,
+       int ge,
+       int i_lt,
+       int i_le,
+       int i_gt,
+       int i_ge,
+       int i_lg,
+       int i_un)
+{
+  if (eq != (a == b))
+    abort ();
+
+  if (ne != (a != b))
+    abort ();
+
+  if (lt != (a < b))
+    abort ();
+
+  if (le != (a <= b))
+    abort ();
+
+  if (gt != (a > b))
+    abort ();
+
+  if (ge != (a >= b))
+    abort ();
+
+  if (i_lt != __builtin_isless (a, b))
+    abort ();
+
+  if (i_le != __builtin_islessequal (a, b))
+    abort ();
+
+  if (i_gt != __builtin_isgreater (a, b))
+    abort ();
+
+  if (i_ge != __builtin_isgreaterequal (a, b))
+    abort ();
+
+  if (i_lg != __builtin_islessgreater (a, b))
+    abort ();
+
+  if (i_un != __builtin_isunordered (a, b))
+    abort ();
+}
+
+int main (void)
+{
+  TYPE one   = (TYPE) +1.0;
+  TYPE two   = (TYPE) +2.0;
+  TYPE pzero = (TYPE) +0.0;
+  TYPE mzero = (TYPE) -0.0;
+  TYPE nan   = (TYPE) NAN;
+  TYPE snan  = (TYPE) SNAN;
+
+  check (one,   two,   0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 1, 0);
+  check (one,   one,   1, 0, 0, 1, 0, 1, 0, 1, 0, 1, 0, 0);
+  check (one,   pzero, 0, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 0);
+  check (mzero, pzero, 1, 0, 0, 1, 0, 1, 0, 1, 0, 1, 0, 0);
+  check (nan,   one,   0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  check (one,   nan,   0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  check (nan,   nan,   0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  check (snan,  one,   0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  check (one,   snan,  0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  check (snan,  nan,   0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  check (nan,   snan,  0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
+  return 0;
+}