This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

neg.fmt and abs.fmt on MIPS


The MIPS ISAs define "neg.fmt" and "abs.fmt" to be arithmetic
instructions.  They treat all NaN inputs as invalid and do not change
the NaNs' sign bits.  gcc nevertheless implements "-f" using "neg.fmt"
and "fabs*()" using "abs.fmt".

I suppose it's open to debate whether this is a problem or not.
Appendix A of the IEEE standard recommends that negation and absolute
operations apply to NaNs too, although somewhat indirectly in the
latter casse:

  1. Copysign(x, y) returns x with the sign of y. Hence, abs(x) =
     copysign(x, 1.0), even if x is NaN.

  2. ­x is x copied with its sign reversed, not 0 ­ x; the distinction is
     germane when x is ±0 or NaN. Consequently, it is a mistake to use
     the sign bit to distinguish signaling NaNs from quiet NaNs.

(It also says:

  Some functions, such as the copy operation y := x without change of
  format, may at the implementor's option be treated as nonarithmetic
  operations which do not signal the invalid operation exception for
  signaling NaNs; the functions in question are (1), (2), (6), and (7).

but the problem here applies to quiet NaNs too.)

However, the appendix is not normative, so it could be argued that
gcc's behaviour is OK.  The counter argument is that people who _do_
want the appendix's recommended behaviour have no way to select it.

If we provide an option to control the whether the sign of NaNs matters,
the question then is: what should the default be?  Keeping the current
behaviour by default might be less surprising to long-standing users,
but on the other hand, not having traditional IEEE behaviour by default
might be surprising too.  GCC has generally tried to stick to the
standard (appendix and all) by default, with options to relax the rules
in certain ways.

The issue is more important than it used to be because of copysign().
GCC now treats copysign() as a built-in function and expects it to be
equivalent of "x == fabs (x) ? x : -x".  This equivalence does not
hold if fabs() and -f are implemented using the FPU instructions
and copysign() has therefore regressed from earlier versions of gcc.

The options seem to be:

  (1) Say "not normative, don't care" for -f and fabs() and copysign().

  (2) Say "not normative, don't care" for -f and fabs() only, but try to
      make copysign() handle NaNs as the appendiex suggests.  This was
      gcc's behaviour before copysign() became a built-in function.

  (3) As for (1) or (2), but have a target-specific option to do what
      the appendix suggests.

  (4) Make all three operations do what the appendix suggests and use
      options to relax the rules.

I think (1) is bad for the reason given above: users who want the
appendix's suggested behaviour have no way to get it.  I think (2)
is far too inconsistent.  (3) is possible, but like I say above,
is opposite to what one would normally expect on MIPS.  gcc has
traditionally generated IEEE code by default.

I therefore prefer (4).  We could add a target-specific option to allow
-f, fabs() and copysign() to be implemented as they are now _but without
making other changes to the rules_, but unless someone has a particular
need for such an option, I think it would be overkill.  It would be yet
another permutation to consider.  I think we should instead just base the
check on HONOR_NANS and make the decision subject to options like
-ffinite-math-only.

I'd like to apply this patch in the next few days, but I realise it
might be controversial, so I'll wait to see if there's any feedback
first.  I'm afraid I've been sitting on this for a long time, uming
and ahing about what the right thing to do is.

Tested on mipsisa64-elf, options:

   {-EB,-EL}{-mips64,-mips32}{-mhard-float,-msoft-float}

Richard


gcc/
	* config/mips/mips.c (CODE_FOR_mips_abs_ps): Delete.
	* config/mips/mips.md (UNSPEC_ABS_PS): New constant.
	(UNSPEC_RSQRT1, UNSPEC_RSQRT2, UNSPEC_RECIP1, UNSPEC_RECIP2)
	(UNSPEC_SINGLE_CC, UNSPEC_SCC): Bump values by 1.
	(*nmadd<mode>, *nmadd<mode>_fastmath): Require !HONOR_NANS.
	(*nmsub<mode>, *nmsub<mode>_fastmath): Likewise.
	(abs<mode>2, neg<mode>2): Likewise.
	* config/mips/mips-ps-3d.md (mips_abs_ps): New define_expand.
	(*mips_abs_ps): New define_insn.

gcc/testsuite/
	* gcc.target/mips/mips-ps-type.c: Add -ffinite-math-only.
	* gcc.target/mips/nmadd-2.c: Likewise.
	* gcc.target/mips/mips-ps-6.c: New test.
	* gcc.target/mips/neg-abs-1.c: Likewise.
	* gcc.target/mips/neg-abs-2.c: Likewise.
	* gcc.target/mips/nmadd-3.c: New test.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 116995)
+++ gcc/config/mips/mips.c	(working copy)
@@ -10084,9 +10084,6 @@ #define CMP_BUILTINS(COND)						\
   CMP_4S_BUILTINS (c, COND),						\
   CMP_4S_BUILTINS (cabs, COND)
 
-/* __builtin_mips_abs_ps() maps to the standard absM2 pattern.  */
-#define CODE_FOR_mips_abs_ps CODE_FOR_absv2sf2
-
 static const struct builtin_description mips_bdesc[] =
 {
   DIRECT_BUILTIN (pll_ps, MIPS_V2SF_FTYPE_V2SF_V2SF, MASK_PAIRED_SINGLE_FLOAT),
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 116995)
+++ gcc/config/mips/mips.md	(working copy)
@@ -67,13 +67,14 @@ (define_constants
    (UNSPEC_CVT_PW_PS		205)
    (UNSPEC_CVT_PS_PW		206)
    (UNSPEC_MULR_PS		207)
+   (UNSPEC_ABS_PS		208)
 
-   (UNSPEC_RSQRT1		208)
-   (UNSPEC_RSQRT2		209)
-   (UNSPEC_RECIP1		210)
-   (UNSPEC_RECIP2		211)
-   (UNSPEC_SINGLE_CC		212)
-   (UNSPEC_SCC			213)
+   (UNSPEC_RSQRT1		209)
+   (UNSPEC_RSQRT2		210)
+   (UNSPEC_RECIP1		211)
+   (UNSPEC_RECIP2		212)
+   (UNSPEC_SINGLE_CC		213)
+   (UNSPEC_SCC			214)
 
    ;; MIPS DSP ASE Revision 0.98 3/24/2005
    (UNSPEC_ADDQ			300)
@@ -1765,7 +1766,8 @@ (define_insn "*nmadd<mode>"
 			      (match_operand:ANYF 2 "register_operand" "f"))
 		   (match_operand:ANYF 3 "register_operand" "f"))))]
   "ISA_HAS_NMADD_NMSUB && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)"
+   && HONOR_SIGNED_ZEROS (<MODE>mode)
+   && !HONOR_NANS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -1777,7 +1779,8 @@ (define_insn "*nmadd<mode>_fastmath"
 		    (match_operand:ANYF 2 "register_operand" "f"))
 	 (match_operand:ANYF 3 "register_operand" "f")))]
   "ISA_HAS_NMADD_NMSUB && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)
+   && !HONOR_NANS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -1789,7 +1792,8 @@ (define_insn "*nmsub<mode>"
 			      (match_operand:ANYF 3 "register_operand" "f"))
 		   (match_operand:ANYF 1 "register_operand" "f"))))]
   "ISA_HAS_NMADD_NMSUB && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)"
+   && HONOR_SIGNED_ZEROS (<MODE>mode)
+   && !HONOR_NANS (<MODE>mode)"
   "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -1801,7 +1805,8 @@ (define_insn "*nmsub<mode>_fastmath"
 	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
 		    (match_operand:ANYF 3 "register_operand" "f"))))]
   "ISA_HAS_NMADD_NMSUB && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)
+   && !HONOR_NANS (<MODE>mode)"
   "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -1972,10 +1977,14 @@ (define_insn "*rsqrt<mode>b"
 ;; Do not use the integer abs macro instruction, since that signals an
 ;; exception on -2147483648 (sigh).
 
+;; abs.fmt is an arithmetic instruction and treats all NaN inputs as
+;; invalid; it does not clear their sign bits.  We therefore can't use
+;; abs.fmt if the signs of NaNs matter.
+
 (define_insn "abs<mode>2"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(abs:ANYF (match_operand:ANYF 1 "register_operand" "f")))]
-  ""
+  "!HONOR_NANS (<MODE>mode)"
   "abs.<fmt>\t%0,%1"
   [(set_attr "type" "fabs")
    (set_attr "mode" "<UNITMODE>")])
@@ -2024,10 +2033,14 @@ (define_insn "negdi2"
   [(set_attr "type"	"arith")
    (set_attr "mode"	"DI")])
 
+;; neg.fmt is an arithmetic instruction and treats all NaN inputs as
+;; invalid; it does not flip their sign bit.  We therefore can't use
+;; neg.fmt if the signs of NaNs matter.
+
 (define_insn "neg<mode>2"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(neg:ANYF (match_operand:ANYF 1 "register_operand" "f")))]
-  ""
+  "!HONOR_NANS (<MODE>mode)"
   "neg.<fmt>\t%0,%1"
   [(set_attr "type" "fneg")
    (set_attr "mode" "<UNITMODE>")])
Index: gcc/config/mips/mips-ps-3d.md
===================================================================
--- gcc/config/mips/mips-ps-3d.md	(revision 116995)
+++ gcc/config/mips/mips-ps-3d.md	(working copy)
@@ -276,6 +276,31 @@ (define_insn "mips_mulr_ps"
   [(set_attr "type" "fmul")
    (set_attr "mode" "SF")])
 
+; abs.ps
+(define_expand "mips_abs_ps"
+  [(set (match_operand:V2SF 0 "register_operand")
+	(unspec:V2SF [(match_operand:V2SF 1 "register_operand")]
+		     UNSPEC_ABS_PS))]
+  "TARGET_PAIRED_SINGLE_FLOAT"
+{
+  /* If we can ignore NaNs, this operation is equivalent to the
+     rtl ABS code.  */
+  if (!HONOR_NANS (V2SFmode))
+    {
+      emit_insn (gen_absv2sf2 (operands[0], operands[1]));
+      DONE;
+    }
+})
+
+(define_insn "*mips_abs_ps"
+  [(set (match_operand:V2SF 0 "register_operand" "=f")
+	(unspec:V2SF [(match_operand:V2SF 1 "register_operand" "f")]
+		     UNSPEC_ABS_PS))]
+  "TARGET_PAIRED_SINGLE_FLOAT"
+  "abs.ps\t%0,%1"
+  [(set_attr "type" "fabs")
+   (set_attr "mode" "SF")])
+
 ;----------------------------------------------------------------------------
 ; Floating Point Comparisons for Scalars
 ;----------------------------------------------------------------------------
Index: gcc/testsuite/gcc.target/mips/mips-ps-type.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mips-ps-type.c	(revision 116995)
+++ gcc/testsuite/gcc.target/mips/mips-ps-type.c	(working copy)
@@ -1,6 +1,7 @@
-/* Test v2sf calculations */
+/* Test v2sf calculations.  The nmadd and nmsub patterns need
+   -ffinite-math-only.  */
 /* { dg-do compile } */ 
-/* { dg-mips-options "-mips64 -O2 -mpaired-single -mhard-float -mgp64" } */
+/* { dg-mips-options "-mips64 -O2 -mpaired-single -mhard-float -mgp64 -ffinite-math-only" } */
 /* { dg-final { scan-assembler "cvt.ps.s" } } */ 
 /* { dg-final { scan-assembler "mov.ps" } } */ 
 /* { dg-final { scan-assembler "ldc1" } } */ 
Index: gcc/testsuite/gcc.target/mips/nmadd-2.c
===================================================================
--- gcc/testsuite/gcc.target/mips/nmadd-2.c	(revision 116995)
+++ gcc/testsuite/gcc.target/mips/nmadd-2.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-mips-options "-O2 -fno-fast-math -mips4 -mhard-float" } */
+/* { dg-mips-options "-O2 -fno-fast-math -ffinite-math-only -mips4 -mhard-float" } */
 /* { dg-final { scan-assembler "nmadd.s" } } */
 /* { dg-final { scan-assembler "nmadd.d" } } */
 /* { dg-final { scan-assembler "nmsub.s" } } */
Index: gcc/testsuite/gcc.target/mips/mips-ps-6.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mips-ps-6.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/mips-ps-6.c	(revision 0)
@@ -0,0 +1,136 @@
+/* mips-ps-2.c with an extra -ffinite-math-only option.  This option
+   changes the way that abs.ps is handled.  */
+/* { dg-do run { target mipsisa64*-*-* } } */
+/* { dg-mips-options "-mips64 -O2 -mpaired-single -mhard-float -mgp64 -ffinite-math-only" } */
+
+/* Test MIPS paired-single builtin functions */
+#include <stdlib.h>
+#include <stdio.h>
+
+typedef float v2sf __attribute__ ((vector_size(8)));
+
+int main ()
+{
+  int little_endian;
+  v2sf a, b, c, d;
+  float e,f;
+  int i;
+
+  union { long long ll; int i[2]; } endianness_test;
+  endianness_test.ll = 1;
+  little_endian = endianness_test.i[0];
+
+  /* pll.ps */
+  a = (v2sf) {1, 2};
+  b = (v2sf) {3, 4};
+  c = __builtin_mips_pll_ps (a, b);
+  if (little_endian) // little endian
+    d = (v2sf) {3, 1};
+  else // big endian
+    d = (v2sf) {2, 4};
+
+  if (!__builtin_mips_upper_c_eq_ps (c, d) ||
+      !__builtin_mips_lower_c_eq_ps (c, d))
+    abort ();
+
+  /* pul.ps */
+  a = (v2sf) {1, 2};
+  b = (v2sf) {3, 4};
+  c = __builtin_mips_pul_ps (a, b);
+  if (little_endian) // little endian
+    d = (v2sf) {3, 2};
+  else // big endian
+    d = (v2sf) {1, 4};
+  if (!__builtin_mips_upper_c_eq_ps (c, d) ||
+      !__builtin_mips_lower_c_eq_ps (c, d))
+    abort ();
+
+  /* plu.ps */
+  a = (v2sf) {1, 2};
+  b = (v2sf) {3, 4};
+  c = __builtin_mips_plu_ps (a, b);
+  if (little_endian) // little endian
+    d = (v2sf) {4, 1};
+  else // big endian
+    d = (v2sf) {2, 3};
+  if (!__builtin_mips_upper_c_eq_ps (c, d) ||
+      !__builtin_mips_lower_c_eq_ps (c, d))
+    abort ();
+
+  /* puu.ps */
+  a = (v2sf) {1, 2};
+  b = (v2sf) {3, 4};
+  c = __builtin_mips_puu_ps (a, b);
+  if (little_endian) // little endian
+    d = (v2sf) {4, 2};
+  else // big endian
+    d = (v2sf) {1, 3};
+  if (!__builtin_mips_upper_c_eq_ps (c, d) ||
+      !__builtin_mips_lower_c_eq_ps (c, d))
+    abort ();
+
+  /* cvt.ps.s */
+  e = 3.4;
+  f = 4.5; 
+  a = __builtin_mips_cvt_ps_s (e, f);
+  if (little_endian) // little endian
+    b = (v2sf) {4.5, 3.4};
+  else // big endian
+    b = (v2sf) {3.4, 4.5};
+  if (!__builtin_mips_upper_c_eq_ps (a, b) ||
+      !__builtin_mips_lower_c_eq_ps (a, b))
+    abort ();
+
+  /* cvt.s.pl */
+  a = (v2sf) {35.1, 120.2};
+  e = __builtin_mips_cvt_s_pl (a);
+  if (little_endian) // little endian
+    f = 35.1; 
+  else // big endian
+    f = 120.2;
+  if (e != f)
+    abort ();
+
+  /* cvt.s.pu */
+  a = (v2sf) {30.0, 100.0};
+  e = __builtin_mips_cvt_s_pu (a);
+  if (little_endian) // little endian
+    f = 100.0;
+  else // big endian
+    f = 30.0; 
+  if (e != f)
+    abort ();
+
+  /* abs.ps */
+  a = (v2sf) {-3.4, -5.8};
+  b = __builtin_mips_abs_ps (a);
+  c = (v2sf) {3.4, 5.8};
+  if (!__builtin_mips_upper_c_eq_ps (b, c) ||
+      !__builtin_mips_lower_c_eq_ps (b, c))
+    abort ();
+
+  /* alnv.ps with rs = 4*/
+  a = (v2sf) {1, 2};
+  b = (v2sf) {3, 4};
+  i = 4;
+  c = __builtin_mips_alnv_ps (a, b, i);
+  d = (v2sf) {2, 3};
+
+  if (!__builtin_mips_upper_c_eq_ps (c, d) ||
+      !__builtin_mips_lower_c_eq_ps (c, d))
+    abort ();
+
+  /* alnv.ps with rs = 0 */
+  a = (v2sf) {5, 6};
+  b = (v2sf) {7, 8};
+  i = 0;
+  c = __builtin_mips_alnv_ps (a, b, i);
+  d = (v2sf) {5, 6};
+
+  if (!__builtin_mips_upper_c_eq_ps (c, d) ||
+      !__builtin_mips_lower_c_eq_ps (c, d))
+    abort ();
+
+  printf ("Test Passes\n");
+  exit (0);
+}
Index: gcc/testsuite/gcc.target/mips/neg-abs-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/neg-abs-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/neg-abs-1.c	(revision 0)
@@ -0,0 +1,13 @@
+/* Make sure that we use abs.fmt and neg.fmt when the signs of NaNs don't
+   matter.  */
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -mhard-float -ffinite-math-only" } */
+/* { dg-final { scan-assembler "neg.s" } } */
+/* { dg-final { scan-assembler "neg.d" } } */
+/* { dg-final { scan-assembler "abs.s" } } */
+/* { dg-final { scan-assembler "abs.d" } } */
+
+float f1 (float f) { return -f; }
+float f2 (float f) { return __builtin_fabsf (f); }
+double d1 (double d) { return -d; }
+double d2 (double d) { return __builtin_fabs (d); }
Index: gcc/testsuite/gcc.target/mips/neg-abs-2.c
===================================================================
--- gcc/testsuite/gcc.target/mips/neg-abs-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/neg-abs-2.c	(revision 0)
@@ -0,0 +1,13 @@
+/* Make sure that we avoid abs.fmt and neg.fmt when the signs of NaNs
+   matter.  */
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -mhard-float -fno-finite-math-only" } */
+/* { dg-final { scan-assembler-not "neg.s" } } */
+/* { dg-final { scan-assembler-not "neg.d" } } */
+/* { dg-final { scan-assembler-not "abs.s" } } */
+/* { dg-final { scan-assembler-not "abs.d" } } */
+
+float f1 (float f) { return -f; }
+float f2 (float f) { return __builtin_fabsf (f); }
+double d1 (double d) { return -d; }
+double d2 (double d) { return __builtin_fabs (d); }
Index: gcc/testsuite/gcc.target/mips/nmadd-3.c
===================================================================
--- gcc/testsuite/gcc.target/mips/nmadd-3.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/nmadd-3.c	(revision 0)
@@ -0,0 +1,32 @@
+/* The same code as nmadd-2.c, but compiled with -fno-finite-math-only.
+   We can't use nmadd and nmsub in that case.  */
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -fno-fast-math -fno-finite-math-only -mips4 -mhard-float" } */
+/* { dg-final { scan-assembler-not "nmadd.s" } } */
+/* { dg-final { scan-assembler-not "nmadd.d" } } */
+/* { dg-final { scan-assembler-not "nmsub.s" } } */
+/* { dg-final { scan-assembler-not "nmsub.d" } } */
+
+float
+sub1 (float f, float g, float h)
+{
+  return -((f * g) + h);
+}
+
+double
+sub2 (double f, double g, double h)
+{
+  return -((f * g) + h);
+}
+
+float
+sub3 (float f, float g, float h)
+{
+  return -((f * g) - h);
+}
+
+double
+sub4 (double f, double g, double h)
+{
+  return -((f * g) - h);
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]