[Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd

public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed

* [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
@ 2015-06-11 20:09 Steve Ellcey 
  2015-06-11 20:39 ` Joseph Myers
  0 siblings, 1 reply; 25+ messages in thread
From: Steve Ellcey  @ 2015-06-11 20:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: clm, matthew.fortune

This patch updates MIPS to handle fused multiply and add instructions using
the standard fp-contract infrastructure.  Currently MIPS handles -mfused-madd
as a target specific option and it is not affected by the -ffp-contract option.

This patch changes -mfused-madd to be an alias for -ffp-contract=fast by
using the standard config/fused-madd.opt option file.  It also adds
fma, fms, fnma, and fnms instructions to be used with convert_mult_to_fma
and updates the definitions of the non-fused multiply and add instructions
that some MIPS architectures support.

After this patch I found the following changes in behaviour:

mips1, mips2, mips3, and mips32[r1] had no change because they have no
fused (or unfused) madd instructions.

loongson and r8000 have the most changes,  they no longer generate msub
instructions with -mfused-madd because that instruction does not generate
the correct NAN in some cases (the sign may be wrong).  If HONOR_NANS
is not set then they will generate msub instructions.

mips4 (other than r8000), mips32r2, mips64[r1], and mips64r2 will
now generated [unfused] multiply and add instructions even if 
-mno-fused-madd (or -ffp-contract=on) are used because these instructions
are not fused and will round in between the two operations.  This means
they give the same result as the individual instructions.

mips32r6 and mips64r6 now obey -mno-fused-madd (or -ffp-contract=on)
whereas before they were generating fused madds even when one of these
options were used.

One thing I did not put in this patch was to add code to use the
mips32r6/mips64r6 msubf instruction.  This instruction implements
'c - (a * b)', not '(a * b) - c' and since it not currently used by
GCC I decided not to add it to this patch.

I did run-time testing with mips32 and mips64, versions 1, 2, and 6.
With other MIPS versions (mips[1234], r8000, loongson) I did
compilations to assembly language and did a visual inspection of the
code to compare the results before and after this patch was applied.

OK to checkin?

Steve Ellcey
sellcey@imgtec.com


2015-06-11  Steve Ellcey  <sellcey@imgtec.com>

	* config.gcc (mips*-*-*): Add fused-madd.opt.
	* config/mips/mips.opt (mfused-madd): Remove.
	* config/mips/mips.c (mips_rtx_costs): Update cost calculations.
	* config/mips/mips.h (TARGET_MIPS8000): New.
	(ISA_HAS_FP_MADD4_MSUB4): Remove.
	(ISA_HAS_FP_MADDF_MSUBF): Remove.
	(ISA_HAS_FP_MADD3_MSUB3): Remove.
	(ISA_HAS_NMADD4_NMSUB4): Remove.
	(ISA_HAS_NMADD3_NMSUB3): Remove.
	(ISA_HAS_FUSED_MADD4): New.
	(ISA_HAS_UNFUSED_MADD4): New.
	(ISA_HAS_FUSED_MADDF): New.
	(ISA_HAS_FUSED_MADD3): New.
	* config/mips/mips.md: (fma<mode>4) Change from insn to expand.
	(*fma<mode>4_madd3) New.
	(*fma<mode>4_madd4) New.
	(*fma<mode>4_maddf) New.
	(fms<mode>4) New.
	(*fms<mode>4_msub3) New.
	(*fms<mode>4_msub4) New.
	(fnma<mode>4) New.
	(*fnma<mode>4_nmadd3) New.
	(*fnma<mode>4_nmadd4) New.
	(fnms<mode>4) New.
	(*fnms<mode>4_nmsub3) New.
	(*fnms<mode>4_nmsub4) New.
	(*madd4<mode>) Modify to be unfused only.
	(*msub4<mode>) Modify to be unfused only.
	(*nmadd4<mode>) Modify to be unfused only.
	(*nmsub4<mode>) Modify to be unfused only.
	(*madd3<mode>) Remove.
	(*msub3<mode>) Remove.
	(*nmadd3<mode>) Remove.
	(*nmsub3<mode>) Remove.
	(*nmadd3<mode>_fastmath) Remove.
	(*nmsub3<mode>_fastmath) Remove.
	(*nmadd4<mode>_fastmath) Update condition.
	(*nmsub4<mode>_fastmath)  Update condition.


diff --git a/gcc/config.gcc b/gcc/config.gcc
index 13a567f..f8c6307 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -418,7 +418,7 @@ microblaze*-*-*)
 mips*-*-*)
 	cpu_type=mips
 	extra_headers="loongson.h"
-	extra_options="${extra_options} g.opt mips/mips-tables.opt"
+	extra_options="${extra_options} g.opt fused-madd.opt mips/mips-tables.opt"
 	;;
 nds32*)
 	cpu_type=nds32
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index e81134e..3ed9804 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -4068,14 +4068,12 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return true;
 
     case MINUS:
-      if (float_mode_p
-	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
-	  && TARGET_FUSED_MADD
-	  && !HONOR_NANS (mode)
-	  && !HONOR_SIGNED_ZEROS (mode))
+      if (float_mode_p && ISA_HAS_UNFUSED_MADD4
+	  && !HONOR_NANS (mode) && !HONOR_SIGNED_ZEROS (mode))
 	{
-	  /* See if we can use NMADD or NMSUB.  See mips.md for the
-	     associated patterns.  */
+	  /* See if we can use NMADD or NMSUB via the *nmadd4<mode>_fastmath
+	     or *nmsub4<mode>_fastmath patterns.  These patterns check for
+	     HONOR_NANS and HONOR_SIGNED_ZEROS so we check here too.  */
 	  rtx op0 = XEXP (x, 0);
 	  rtx op1 = XEXP (x, 1);
 	  if (GET_CODE (op0) == MULT && GET_CODE (XEXP (op0, 0)) == NEG)
@@ -4102,9 +4100,7 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
 	{
 	  /* If this is part of a MADD or MSUB, treat the PLUS as
 	     being free.  */
-	  if ((ISA_HAS_FP_MADD4_MSUB4 || ISA_HAS_FP_MADD3_MSUB3)
-	      && TARGET_FUSED_MADD
-	      && GET_CODE (XEXP (x, 0)) == MULT)
+	  if (ISA_HAS_UNFUSED_MADD4 && GET_CODE (XEXP (x, 0)) == MULT)
 	    *total = 0;
 	  else
 	    *total = mips_cost->fp_add;
@@ -4136,14 +4132,11 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return true;
 
     case NEG:
-      if (float_mode_p
-	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
-	  && TARGET_FUSED_MADD
-	  && !HONOR_NANS (mode)
-	  && HONOR_SIGNED_ZEROS (mode))
+      if (float_mode_p && ISA_HAS_UNFUSED_MADD4 && !HONOR_NANS (mode))
 	{
-	  /* See if we can use NMADD or NMSUB.  See mips.md for the
-	     associated patterns.  */
+	  /* See if we can use NMADD or NMSUB via the *nmadd4<mode> or
+	     *nmsub4<mode> patterns.  These patterns do not check
+	     HONOR_NANS but not HONOR_SIGNED_ZEROS so we check the same.  */
 	  rtx op = XEXP (x, 0);
 	  if ((GET_CODE (op) == PLUS || GET_CODE (op) == MINUS)
 	      && GET_CODE (XEXP (op, 0)) == MULT)
@@ -4163,8 +4156,7 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return false;
 
     case FMA:
-      if (ISA_HAS_FP_MADDF_MSUBF)
-	*total = mips_fp_mult_cost (mode);
+      *total = mips_fp_mult_cost (mode);
       return false;
 
     case MULT:
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index bceef31..7a6f917 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -236,6 +236,7 @@ struct mips_cpu_info {
 #define TARGET_MIPS5500             (mips_arch == PROCESSOR_R5500)
 #define TARGET_MIPS5900             (mips_arch == PROCESSOR_R5900)
 #define TARGET_MIPS7000             (mips_arch == PROCESSOR_R7000)
+#define TARGET_MIPS8000             (mips_arch == PROCESSOR_R8000)
 #define TARGET_MIPS9000             (mips_arch == PROCESSOR_R9000)
 #define TARGET_OCTEON		    (mips_arch == PROCESSOR_OCTEON	\
 				     || mips_arch == PROCESSOR_OCTEON2	\
@@ -998,22 +999,21 @@ struct mips_cpu_info {
 /* Integer multiply-accumulate instructions should be generated.  */
 #define GENERATE_MADD_MSUB	(TARGET_IMADD && !TARGET_MIPS16)
 
-/* ISA has floating-point madd and msub instructions 'd = a * b [+-] c'.  */
-#define ISA_HAS_FP_MADD4_MSUB4  ISA_HAS_FP4
+/* ISA has 4 operand fused madd instructions of the form
+   'd = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_FUSED_MADD4	TARGET_MIPS8000
 
-/* ISA has floating-point MADDF and MSUBF instructions 'd = d [+-] a * b'.  */
-#define ISA_HAS_FP_MADDF_MSUBF  (mips_isa_rev >= 6)
+/* ISA has 4 operand unfused madd instructions of the form
+   'd = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_UNFUSED_MADD4	(ISA_HAS_FP4 && !TARGET_MIPS8000)
 
-/* ISA has floating-point madd and msub instructions 'c = a * b [+-] c'.  */
-#define ISA_HAS_FP_MADD3_MSUB3  TARGET_LOONGSON_2EF
+/* ISA has 3 operand r6 fused madd instructions of the form
+   'c = c [+-] (a * b)'.  */
+#define ISA_HAS_FUSED_MADDF	(mips_isa_rev >= 6)
 
-/* ISA has floating-point nmadd and nmsub instructions
-   'd = -((a * b) [+-] c)'.  */
-#define ISA_HAS_NMADD4_NMSUB4	ISA_HAS_FP4
-
-/* ISA has floating-point nmadd and nmsub instructions
-   'c = -((a * b) [+-] c)'.  */
-#define ISA_HAS_NMADD3_NMSUB3	TARGET_LOONGSON_2EF
+/* ISA has 3 operand loongson fused madd instructions of the form
+   'c = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_FUSED_MADD3	TARGET_LOONGSON_2EF
 
 /* ISA has floating-point RECIP.fmt and RSQRT.fmt instructions.  The
    MIPS64 rev. 1 ISA says that RECIP.D and RSQRT.D are unpredictable when
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 0a23fa2..4b2a6ed 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -2475,165 +2475,238 @@
 
 ;; Floating point multiply accumulate instructions.
 
-(define_insn "*madd4<mode>"
+(define_expand "fma<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand")
+		  (match_operand:ANYF 2 "register_operand")
+		  (match_operand:ANYF 3 "register_operand")))]
+  "ISA_HAS_FUSED_MADDF || ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4")
+
+(define_insn "*fma<mode>4_madd3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_FP_MADD4_MSUB4 && TARGET_FUSED_MADD"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "0")))]
+  "ISA_HAS_FUSED_MADD3"
+  "madd.<fmt>\t%0,%1,%2"
+  [(set_attr "type" "fmadd")
+   (set_attr "mode" "<UNITMODE>")])
+
+(define_insn "*fma<mode>4_madd4"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_FUSED_MADD4"
   "madd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "fma<mode>4"
+(define_insn "*fma<mode>4_maddf"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
 		  (match_operand:ANYF 2 "register_operand" "f")
 		  (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADDF_MSUBF"
+  "ISA_HAS_FUSED_MADDF"
   "maddf.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*madd3<mode>"
+;; The fms, fnma, and fnms instructions cannot be used when HONOR_NANS
+;; is true because they may return a +qNAN when a -qNAN is required by
+;; IEEE 754-2008.
+
+(define_expand "fms<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand")
+		  (match_operand:ANYF 2 "register_operand")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4) && !HONOR_NANS (<MODE>mode)")
+
+(define_insn "*fms<mode>4_msub3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADD3_MSUB3 && TARGET_FUSED_MADD"
-  "madd.<fmt>\t%0,%1,%2"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand" "0"))))]
+  "ISA_HAS_FUSED_MADD3 && !HONOR_NANS (<MODE>mode)"
+  "msub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*msub4<mode>"
+(define_insn "*fms<mode>4_msub4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			       (match_operand:ANYF 2 "register_operand" "f"))
-		    (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_FP_MADD4_MSUB4 && TARGET_FUSED_MADD"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_FUSED_MADD4 && !HONOR_NANS (<MODE>mode)"
   "msub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*msub3<mode>"
+;; fnma is defined in GCC as (fma (neg op1) op2 op3)
+;; (-op1 * op2) + op3 ==> -(op1 * op2) + op3 ==> -((op1 * op2) - op3)
+;; The mips nmsub instructions implement -((op1 * op2) - op3)
+;; This transformation means we may return the wrong signed zero
+;; so we check HONOR_SIGNED_ZEROS.
+
+(define_expand "fnma<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand"))
+		  (match_operand:ANYF 2 "register_operand")
+		  (match_operand:ANYF 3 "register_operand")))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
+   && !HONOR_NANS (<MODE>mode) && !HONOR_SIGNED_ZEROS (<MODE>mode)")
+
+(define_insn "*fnma<mode>4_nmsub3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			       (match_operand:ANYF 2 "register_operand" "f"))
-		    (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADD3_MSUB3 && TARGET_FUSED_MADD"
-  "msub.<fmt>\t%0,%1,%2"
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "0")))]
+  "ISA_HAS_FUSED_MADD3
+   && !HONOR_NANS (<MODE>mode) && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd4<mode>"
+(define_insn "*fnma<mode>4_nmsub4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (plus:ANYF
-		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmadd.<fmt>\t%0,%3,%1,%2"
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_FUSED_MADD4
+   && !HONOR_NANS (<MODE>mode) && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd3<mode>"
+;; fnms is defined as: (fma (neg op1) op2 (neg op3))
+;; ((-op1) * op2) - op3 ==> -(op1 * op2) - op3 ==> -((op1 * op2) + op3)
+;; The mips nmadd instructions implement -((op1 * op2) + op3)
+;; This transformation means we may return the wrong signed zero
+;; so we check HONOR_SIGNED_ZEROS.
+
+(define_expand "fnms<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand"))
+	  (match_operand:ANYF 2 "register_operand")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
+   && !HONOR_NANS (<MODE>mode) && !HONOR_SIGNED_ZEROS (<MODE>mode)")
+
+(define_insn "*fnms<mode>4_nmadd3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (plus:ANYF
-		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+	  (match_operand:ANYF 2 "register_operand" "f")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand" "0"))))]
+  "ISA_HAS_FUSED_MADD3
+   && !HONOR_NANS (<MODE>mode) && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd4<mode>_fastmath"
+(define_insn "*fnms<mode>4_nmadd4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF
-	 (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
-		    (match_operand:ANYF 2 "register_operand" "f"))
-	 (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+	  (match_operand:ANYF 2 "register_operand" "f")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_FUSED_MADD4
+   && !HONOR_NANS (<MODE>mode) && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd3<mode>_fastmath"
+;; Non-fused Floating point multiply accumulate instructions.
+
+;; These instructions are not fused and round in between the multiply
+;; and the add (or subtract) so they are equivalent to the separate
+;; multiply and add/sub instructions.
+
+(define_insn "*madd4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF
-	 (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
-		    (match_operand:ANYF 2 "register_operand" "f"))
-	 (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmadd.<fmt>\t%0,%1,%2"
+	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "madd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub4<mode>"
+(define_insn "*msub4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (minus:ANYF
-		   (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-			      (match_operand:ANYF 3 "register_operand" "f"))
-		   (match_operand:ANYF 1 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2,%3"
+	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			       (match_operand:ANYF 2 "register_operand" "f"))
+		    (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "msub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub3<mode>"
+;; We need to check for HONOR_NANS on nmadd4 and nmsub4 because the nmadd and
+;; nmsub instructions will not negate qNaN the way IEEE 754-2008 expects.
+;; i.e. (neg (qNAN)) would result in qNaN instead of -qNaN
+
+(define_insn "*nmadd4<mode>"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(neg:ANYF (plus:ANYF
+		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4 && !HONOR_NANS (<MODE>mode)"
+  "nmadd.<fmt>\t%0,%3,%1,%2"
+  [(set_attr "type" "fmadd")
+   (set_attr "mode" "<UNITMODE>")])
+
+(define_insn "*nmsub4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(neg:ANYF (minus:ANYF
-		   (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-			      (match_operand:ANYF 3 "register_operand" "f"))
-		   (match_operand:ANYF 1 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2"
+		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4 && !HONOR_NANS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub4<mode>_fastmath"
+;; Fast-math Non-fused Floating point multiply accumulate instructions.
+
+;; These instructions are not fused but the expressions they match are
+;; not exactly what the instruction implements in the sense that they
+;; may not generate the properly signed zeros or NaNs.
+
+;; This instruction recognizes  ((-op1) * op2) - op3 and generates an
+;; nmadd which is really -((op1 * op2) + op3).  They are equivalent
+;; except for the sign bit when the result is zero or NaN.
+
+(define_insn "*nmadd4<mode>_fastmath"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(minus:ANYF
-	 (match_operand:ANYF 1 "register_operand" "f")
-	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-		    (match_operand:ANYF 3 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
+	  (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		     (match_operand:ANYF 2 "register_operand" "f"))
+	  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4
    && !HONOR_SIGNED_ZEROS (<MODE>mode)
    && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2,%3"
+  "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub3<mode>_fastmath"
+;; This instruction recognizes (op1 - (op2 * op3) and generates an
+;; nmsub which is really -((op2 * op3) - op1).  They are equivalent
+;; except for the sign bit when the result is zero or NaN.
+
+(define_insn "*nmsub4<mode>_fastmath"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(minus:ANYF
-	 (match_operand:ANYF 1 "register_operand" "f")
-	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-		    (match_operand:ANYF 3 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
+	  (match_operand:ANYF 1 "register_operand" "f")
+	  (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
+		     (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4
    && !HONOR_SIGNED_ZEROS (<MODE>mode)
    && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2"
+  "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index a9baebe..348c6e0 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -209,10 +209,6 @@ mflush-func=
 Target RejectNegative Joined Var(mips_cache_flush_func) Init(CACHE_FLUSH_FUNC)
 -mflush-func=FUNC	Use FUNC to flush the cache before calling stack trampolines
 
-mfused-madd
-Target Report Var(TARGET_FUSED_MADD) Init(1)
-Generate floating-point multiply-add instructions
-
 mabs=
 Target RejectNegative Joined Enum(mips_ieee_754_value) Var(mips_abs) Init(MIPS_IEEE_754_DEFAULT)
 -mabs=MODE	Select the IEEE 754 ABS/NEG instruction execution mode

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-11 20:09 [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd Steve Ellcey 
@ 2015-06-11 20:39 ` Joseph Myers
  2015-06-15 20:52   ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Joseph Myers @ 2015-06-11 20:39 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches, clm, matthew.fortune

On Thu, 11 Jun 2015, Steve Ellcey  wrote:

> loongson and r8000 have the most changes,  they no longer generate msub
> instructions with -mfused-madd because that instruction does not generate
> the correct NAN in some cases (the sign may be wrong).  If HONOR_NANS
> is not set then they will generate msub instructions.

There's no such thing as a correct NaN sign for fused multiply-add (at the 
C / GIMPLE / RTL level, that is; there may be a correct sign at the level 
of semantics for processor instructions).  IEEE 754 only specifies signs 
of NaNs for a few operations (copy, negate, abs, copySign).  So while you 
need to avoid negate / abs instructions that don't work properly on NaNs, 
if signs of NaNs are the only concern then that's not a reason to avoid 
any other arithmetic operations.

> One thing I did not put in this patch was to add code to use the
> mips32r6/mips64r6 msubf instruction.  This instruction implements
> 'c - (a * b)', not '(a * b) - c' and since it not currently used by
> GCC I decided not to add it to this patch.

Fused c - (a * b) is exactly equivalent to fused (-a * b) + c or 
(a * -b) + c (I don't know which is canonical in RTL).  (It's *not* 
equivalent to fused -((a * b) - c), when the result is an exact zero.  
And moving negation inside multiplication like that is only valid for a 
fused operation, not unfused if -frounding-math.)

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-11 20:39 ` Joseph Myers
@ 2015-06-15 20:52   ` Maciej W. Rozycki
  2015-06-15 21:37     ` Joseph Myers
  0 siblings, 1 reply; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-15 20:52 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Steve Ellcey, gcc-patches, Catherine Moore, Matthew Fortune

On Thu, 11 Jun 2015, Joseph Myers wrote:

> > loongson and r8000 have the most changes,  they no longer generate msub
> > instructions with -mfused-madd because that instruction does not generate
> > the correct NAN in some cases (the sign may be wrong).  If HONOR_NANS
> > is not set then they will generate msub instructions.
> 
> There's no such thing as a correct NaN sign for fused multiply-add (at the 
> C / GIMPLE / RTL level, that is; there may be a correct sign at the level 
> of semantics for processor instructions).  IEEE 754 only specifies signs 
> of NaNs for a few operations (copy, negate, abs, copySign).  So while you 
> need to avoid negate / abs instructions that don't work properly on NaNs, 
> if signs of NaNs are the only concern then that's not a reason to avoid 
> any other arithmetic operations.

 You are right about fused multiply-add (FMA) as far as IEEE Std 754-2008 
is concerned, however these instructions implement fused multiply-subtract 
(FMS) which is an operation that hasn't been defined by IEEE Std 754-2008 
and neither is expressed by GCC at the RTL level (there's only (fma:M OP1 
OP2 OP3); there's no (fms:M OP1 OP2 OP3) or suchlike).

 Consequently expander patterns like `fmsM4', `fnmaM4', `fnmsM4' and any 
further ones that might be invented for similar operations, that e.g. 
reverse the subtraction, are built around FMA with some of its input 
operands negated.  That negation, implemented with the IEEE Std 754-2008 
`negate' operation that you referred to, by definition is required to 
operate on the sign of its operand in a specific way even if the operand 
is a qNaN.

 So for example `fmsM4', that is specified at the RTL level as (fma:M OP1 
OP2 (neg:M OP3)) will not produce the correct result with the fused 
version of the MIPS MSUB.fmt instruction in the case where OP1 and OP2 are 
numeric data patterns and OP3 is a qNaN data pattern that has its sign bit 
clear.  As specified by IEEE Std 754-2008 the (neg:M OP3) operation is 
required to invert the sign bit of the qNaN data pattern in calculating 
TMP3, and then the (fma:M OP1 OP2 TMP3) operation is required to pass the 
TMP3 qNaN data pattern unchanged in calculating the final result.

> > One thing I did not put in this patch was to add code to use the
> > mips32r6/mips64r6 msubf instruction.  This instruction implements
> > 'c - (a * b)', not '(a * b) - c' and since it not currently used by
> > GCC I decided not to add it to this patch.
> 
> Fused c - (a * b) is exactly equivalent to fused (-a * b) + c or 
> (a * -b) + c (I don't know which is canonical in RTL).  (It's *not* 
> equivalent to fused -((a * b) - c), when the result is an exact zero.  
> And moving negation inside multiplication like that is only valid for a 
> fused operation, not unfused if -frounding-math.)

 Well as I say, IEEE Std 754-2008 does not define a fused (c - (a * b)) or 
fused ((-a * b) + c) operation.  The only fused operation defined by the 
standard is ((a * b) + c) and consequently, as I noted above, any 
additional IEEE Std 754-2008 compliant fused operations have to be 
implemented in terms of negating one or more of input operands, or the 
result.  That is at least my understanding of IEEE Std 754-2008 -- if you 
think otherwise, can you prove me wrong?

 When going beyond IEEE Std 754-2008 we can of course define anything we 
want, but I think that'd have to be controlled with a separate compilation 
flag, assuming that we do want to go there (beyond -ffinite-math-only that 
we already have and that in turn causes HONOR_NANS to return FALSE).

 One complication however is the C11 language standard is still based 
around IEEE Std 754-1985 that in turn does not define any fused operation 
at all.  Question might be whether we want to go into the IEEE Std 
754-2008 territory before (unless?) the C language standard has reached 
there.  However GCC has already adopted FMA, so I gather that we do.  I 
can't speak of other languages GCC supports and their correspondence to 
IEEE Std 754.

 I agree with your note about the sign of exact zero results BTW, but it 
has been taken into account with Steve's patch.  The relevant operations 
use or omit a HONOR_SIGNED_ZEROS check as applicable.

 If I got anything wrong in the above consideration, then I'll be happy to 
be corrected.

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-15 20:52   ` Maciej W. Rozycki
@ 2015-06-15 21:37     ` Joseph Myers
  2015-06-15 22:00       ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Joseph Myers @ 2015-06-15 21:37 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Steve Ellcey, gcc-patches, Catherine Moore, Matthew Fortune

On Mon, 15 Jun 2015, Maciej W. Rozycki wrote:

> operands negated.  That negation, implemented with the IEEE Std 754-2008 
> `negate' operation that you referred to, by definition is required to 
> operate on the sign of its operand in a specific way even if the operand 
> is a qNaN.
> 
>  So for example `fmsM4', that is specified at the RTL level as (fma:M OP1 
> OP2 (neg:M OP3)) will not produce the correct result with the fused 
> version of the MIPS MSUB.fmt instruction in the case where OP1 and OP2 are 
> numeric data patterns and OP3 is a qNaN data pattern that has its sign bit 
> clear.  As specified by IEEE Std 754-2008 the (neg:M OP3) operation is 
> required to invert the sign bit of the qNaN data pattern in calculating 
> TMP3, and then the (fma:M OP1 OP2 TMP3) operation is required to pass the 
> TMP3 qNaN data pattern unchanged in calculating the final result.

It is only required (well, recommended) to pass the *payload*.  The sign 
bit is not part of the payload.  "For all other operations, this standard 
does not specify the sign bit of a NaN result, even when there is only one 
input NaN, or when the NaN is produced from an invalid operation.".

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-15 21:37     ` Joseph Myers
@ 2015-06-15 22:00       ` Maciej W. Rozycki
  2015-06-15 22:09         ` Joseph Myers
  0 siblings, 1 reply; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-15 22:00 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Steve Ellcey, gcc-patches, Catherine Moore, Matthew Fortune

On Mon, 15 Jun 2015, Joseph Myers wrote:

> > operands negated.  That negation, implemented with the IEEE Std 754-2008 
> > `negate' operation that you referred to, by definition is required to 
> > operate on the sign of its operand in a specific way even if the operand 
> > is a qNaN.
> > 
> >  So for example `fmsM4', that is specified at the RTL level as (fma:M OP1 
> > OP2 (neg:M OP3)) will not produce the correct result with the fused 
> > version of the MIPS MSUB.fmt instruction in the case where OP1 and OP2 are 
> > numeric data patterns and OP3 is a qNaN data pattern that has its sign bit 
> > clear.  As specified by IEEE Std 754-2008 the (neg:M OP3) operation is 
> > required to invert the sign bit of the qNaN data pattern in calculating 
> > TMP3, and then the (fma:M OP1 OP2 TMP3) operation is required to pass the 
> > TMP3 qNaN data pattern unchanged in calculating the final result.
> 
> It is only required (well, recommended) to pass the *payload*.  The sign 
> bit is not part of the payload.  "For all other operations, this standard 
> does not specify the sign bit of a NaN result, even when there is only one 
> input NaN, or when the NaN is produced from an invalid operation.".

 However elsewhere: "For an operation with quiet NaN inputs, other than 
maximum and minimum operations, if a floating-point result is to be 
delivered the result shall be a quiet NaN which should be one of the input 
NaNs.".

 I think such a wording makes it clear that the input NaN bit pattern is 
propagated with no change whatsoever and I can't immediately infer if, for 
the purpose of standard interpretation, the clause you've quoted extends 
one that I have or whether one I've quoted narrows one that you have.  
Maybe this is a mistake/inconsistency in the standard.

 Of course you're right these are recommendations only ("should" vs 
"shall"), but I think we might want to have/keep a mode where IEEE Std 754 
recommendations are strictly followed (where hardware permits).

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-15 22:00       ` Maciej W. Rozycki
@ 2015-06-15 22:09         ` Joseph Myers
  2015-06-16 12:19           ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Joseph Myers @ 2015-06-15 22:09 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Steve Ellcey, gcc-patches, Catherine Moore, Matthew Fortune

On Mon, 15 Jun 2015, Maciej W. Rozycki wrote:

> > It is only required (well, recommended) to pass the *payload*.  The sign 
> > bit is not part of the payload.  "For all other operations, this standard 
> > does not specify the sign bit of a NaN result, even when there is only one 
> > input NaN, or when the NaN is produced from an invalid operation.".
> 
>  However elsewhere: "For an operation with quiet NaN inputs, other than 
> maximum and minimum operations, if a floating-point result is to be 
> delivered the result shall be a quiet NaN which should be one of the input 
> NaNs.".

See <http://grouper.ieee.org/groups/754/email/msg03893.html>: "The intent 
is that NaNs which differ only in the sign bit are considered equivalent 
for the purposes of 6.2.".

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-15 22:09         ` Joseph Myers
@ 2015-06-16 12:19           ` Maciej W. Rozycki
  2015-06-16 12:26             ` Joseph Myers
  0 siblings, 1 reply; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-16 12:19 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Steve Ellcey, Richard Sandiford, gcc-patches, Catherine Moore,
	Matthew Fortune

On Mon, 15 Jun 2015, Joseph Myers wrote:

> > > It is only required (well, recommended) to pass the *payload*.  The sign 
> > > bit is not part of the payload.  "For all other operations, this standard 
> > > does not specify the sign bit of a NaN result, even when there is only one 
> > > input NaN, or when the NaN is produced from an invalid operation.".
> > 
> >  However elsewhere: "For an operation with quiet NaN inputs, other than 
> > maximum and minimum operations, if a floating-point result is to be 
> > delivered the result shall be a quiet NaN which should be one of the input 
> > NaNs.".
> 
> See <http://grouper.ieee.org/groups/754/email/msg03893.html>: "The intent 
> is that NaNs which differ only in the sign bit are considered equivalent 
> for the purposes of 6.2.".

 Fair enough, thanks for the pointer.

 This makes me wonder however what the point has been to specify a strict 
particular semantics for the copy, negate, abs, copySign operations with 
respect to the sign bit of qNaN data where any other operation can then 
change the bit in a random fashion.  Do you happen to know what the 
rationale and any possible use cases considered have been?

 Furthermore these checks were deliberately introduced by Richard with his 
proposal here <http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00682.html> 
and agreed upon in the discussion even before IEEE Std 754-2008 has been 
made.  Are you suggesting that the arguments used there, that have led to 
the current arrangement, no longer stand and consequently the HONOR_NANS 
checks introduced are now best dropped?

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-16 12:19           ` Maciej W. Rozycki
@ 2015-06-16 12:26             ` Joseph Myers
  2015-06-16 13:28               ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Joseph Myers @ 2015-06-16 12:26 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Steve Ellcey, Richard Sandiford, gcc-patches, Catherine Moore,
	Matthew Fortune

On Tue, 16 Jun 2015, Maciej W. Rozycki wrote:

>  This makes me wonder however what the point has been to specify a strict 
> particular semantics for the copy, negate, abs, copySign operations with 
> respect to the sign bit of qNaN data where any other operation can then 
> change the bit in a random fashion.  Do you happen to know what the 
> rationale and any possible use cases considered have been?

I don't know.

>  Furthermore these checks were deliberately introduced by Richard with his 
> proposal here <http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00682.html> 
> and agreed upon in the discussion even before IEEE Std 754-2008 has been 
> made.  Are you suggesting that the arguments used there, that have led to 
> the current arrangement, no longer stand and consequently the HONOR_NANS 
> checks introduced are now best dropped?

Only the checks for abs and neg patterns are necessary, not those for 
fused operations.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-16 12:26             ` Joseph Myers
@ 2015-06-16 13:28               ` Maciej W. Rozycki
  2015-06-16 16:13                 ` Joseph Myers
  2015-06-17 10:43                 ` Richard Sandiford
  0 siblings, 2 replies; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-16 13:28 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Steve Ellcey, Richard Sandiford, gcc-patches, Catherine Moore,
	Matthew Fortune

On Tue, 16 Jun 2015, Joseph Myers wrote:

> >  Furthermore these checks were deliberately introduced by Richard with his 
> > proposal here <http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00682.html> 
> > and agreed upon in the discussion even before IEEE Std 754-2008 has been 
> > made.  Are you suggesting that the arguments used there, that have led to 
> > the current arrangement, no longer stand and consequently the HONOR_NANS 
> > checks introduced are now best dropped?
> 
> Only the checks for abs and neg patterns are necessary, not those for 
> fused operations.

 And neither for the unfused combined operations we handle for some MIPS 
processors that implement them presumably?

 In that case I think the HONOR_NANS checks will best be globally removed 
first (where applicable of course), with a separate preparatory change.

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-16 13:28               ` Maciej W. Rozycki
@ 2015-06-16 16:13                 ` Joseph Myers
  2015-06-17 10:43                 ` Richard Sandiford
  1 sibling, 0 replies; 25+ messages in thread
From: Joseph Myers @ 2015-06-16 16:13 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Steve Ellcey, Richard Sandiford, gcc-patches, Catherine Moore,
	Matthew Fortune

On Tue, 16 Jun 2015, Maciej W. Rozycki wrote:

> On Tue, 16 Jun 2015, Joseph Myers wrote:
> 
> > >  Furthermore these checks were deliberately introduced by Richard with his 
> > > proposal here <http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00682.html> 
> > > and agreed upon in the discussion even before IEEE Std 754-2008 has been 
> > > made.  Are you suggesting that the arguments used there, that have led to 
> > > the current arrangement, no longer stand and consequently the HONOR_NANS 
> > > checks introduced are now best dropped?
> > 
> > Only the checks for abs and neg patterns are necessary, not those for 
> > fused operations.
> 
>  And neither for the unfused combined operations we handle for some MIPS 
> processors that implement them presumably?

Indeed.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-16 13:28               ` Maciej W. Rozycki
  2015-06-16 16:13                 ` Joseph Myers
@ 2015-06-17 10:43                 ` Richard Sandiford
  2015-06-17 17:45                   ` Steve Ellcey
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2015-06-17 10:43 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Joseph Myers, Steve Ellcey, gcc-patches, Catherine Moore,
	Matthew Fortune

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
>  In that case I think the HONOR_NANS checks will best be globally removed 
> first (where applicable of course), with a separate preparatory change.

With a comment though :-)  I.e. say that although NEG is the IEEE negate
operation, we don't need to honour NaNs in the unfused combiner patterns
because the rtx operations with which they're being combined don't preserve
the signs of NaNs.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 10:43                 ` Richard Sandiford
@ 2015-06-17 17:45                   ` Steve Ellcey
  2015-06-17 18:44                     ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Steve Ellcey @ 2015-06-17 17:45 UTC (permalink / raw)
  To: Richard Sandiford, Maciej W. Rozycki
  Cc: Joseph Myers, gcc-patches, Catherine Moore, Matthew Fortune

On Wed, 2015-06-17 at 11:41 +0100, Richard Sandiford wrote:
> "Maciej W. Rozycki" <macro@linux-mips.org> writes:
> >  In that case I think the HONOR_NANS checks will best be globally removed 
> > first (where applicable of course), with a separate preparatory change.
> 
> With a comment though :-)  I.e. say that although NEG is the IEEE negate
> operation, we don't need to honour NaNs in the unfused combiner patterns
> because the rtx operations with which they're being combined don't preserve
> the signs of NaNs.
> 
> Thanks,
> Richard

Well, I don't mind removing the HONOR_NAN checks from the MIPS code in
my patch but I am not sure I can do a patch to remove it from the shared
code.  I see about 80 HONOR_NAN checks in the shared code and I am not
sure which ones can and cannot be removed.

Is there any reason why my patch (minus the HONOR_NAN checks) would have
to wait for the other changes?  Here is a new copy with the HONOR_NAN
checks removed from the fused and unfused fma style instructions (and
with comments to explain why it is not needed).

Steve Ellcey
sellcey@imgtec.com



2015-06-17  Steve Ellcey  <sellcey@imgtec.com>

	* config.gcc (mips*-*-*): Add fused-madd.opt.
	* config/mips/mips.opt (mfused-madd): Remove.
	* config/mips/mips.c (mips_rtx_costs): Update cost calculations.
	* config/mips/mips.h (TARGET_MIPS8000): New.
	(ISA_HAS_FP_MADD4_MSUB4): Remove.
	(ISA_HAS_FP_MADDF_MSUBF): Remove.
	(ISA_HAS_FP_MADD3_MSUB3): Remove.
	(ISA_HAS_NMADD4_NMSUB4): Remove.
	(ISA_HAS_NMADD3_NMSUB3): Remove.
	(ISA_HAS_FUSED_MADD4): New.
	(ISA_HAS_UNFUSED_MADD4): New.
	(ISA_HAS_FUSED_MADDF): New.
	(ISA_HAS_FUSED_MADD3): New.
	* config/mips/mips.md: (fma<mode>4) Change from insn to expand.
	(*fma<mode>4_madd3) New.
	(*fma<mode>4_madd4) New.
	(*fma<mode>4_maddf) New.
	(fms<mode>4) New.
	(*fms<mode>4_msub3) New.
	(*fms<mode>4_msub4) New.
	(fnma<mode>4) New.
	(*fnma<mode>4_nmadd3) New.
	(*fnma<mode>4_nmadd4) New.
	(fnms<mode>4) New.
	(*fnms<mode>4_nmsub3) New.
	(*fnms<mode>4_nmsub4) New.
	(*madd4<mode>) Modify to be unfused only.
	(*msub4<mode>) Modify to be unfused only.
	(*nmadd4<mode>) Modify to be unfused only.
	(*nmsub4<mode>) Modify to be unfused only.
	(*madd3<mode>) Remove.
	(*msub3<mode>) Remove.
	(*nmadd3<mode>) Remove.
	(*nmsub3<mode>) Remove.
	(*nmadd3<mode>_fastmath) Remove.
	(*nmsub3<mode>_fastmath) Remove.
	(*nmadd4<mode>_fastmath) Update condition.
	(*nmsub4<mode>_fastmath)  Update condition.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 13a567f..f8c6307 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -418,7 +418,7 @@ microblaze*-*-*)
 mips*-*-*)
 	cpu_type=mips
 	extra_headers="loongson.h"
-	extra_options="${extra_options} g.opt mips/mips-tables.opt"
+	extra_options="${extra_options} g.opt fused-madd.opt mips/mips-tables.opt"
 	;;
 nds32*)
 	cpu_type=nds32
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index e81134e..2d1a233 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -4068,14 +4068,11 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return true;
 
     case MINUS:
-      if (float_mode_p
-	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
-	  && TARGET_FUSED_MADD
-	  && !HONOR_NANS (mode)
-	  && !HONOR_SIGNED_ZEROS (mode))
+      if (float_mode_p && ISA_HAS_UNFUSED_MADD4 && !HONOR_SIGNED_ZEROS (mode))
 	{
-	  /* See if we can use NMADD or NMSUB.  See mips.md for the
-	     associated patterns.  */
+	  /* See if we can use NMADD or NMSUB via the *nmadd4<mode>_fastmath
+	     or *nmsub4<mode>_fastmath patterns.  These patterns check for
+	     HONOR_SIGNED_ZEROS so we check here too.  */
 	  rtx op0 = XEXP (x, 0);
 	  rtx op1 = XEXP (x, 1);
 	  if (GET_CODE (op0) == MULT && GET_CODE (XEXP (op0, 0)) == NEG)
@@ -4102,9 +4099,7 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
 	{
 	  /* If this is part of a MADD or MSUB, treat the PLUS as
 	     being free.  */
-	  if ((ISA_HAS_FP_MADD4_MSUB4 || ISA_HAS_FP_MADD3_MSUB3)
-	      && TARGET_FUSED_MADD
-	      && GET_CODE (XEXP (x, 0)) == MULT)
+	  if (ISA_HAS_UNFUSED_MADD4 && GET_CODE (XEXP (x, 0)) == MULT)
 	    *total = 0;
 	  else
 	    *total = mips_cost->fp_add;
@@ -4136,14 +4131,10 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return true;
 
     case NEG:
-      if (float_mode_p
-	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
-	  && TARGET_FUSED_MADD
-	  && !HONOR_NANS (mode)
-	  && HONOR_SIGNED_ZEROS (mode))
+      if (float_mode_p && ISA_HAS_UNFUSED_MADD4)
 	{
-	  /* See if we can use NMADD or NMSUB.  See mips.md for the
-	     associated patterns.  */
+	  /* See if we can use NMADD or NMSUB via the *nmadd4<mode> or
+	     *nmsub4<mode> patterns.  */
 	  rtx op = XEXP (x, 0);
 	  if ((GET_CODE (op) == PLUS || GET_CODE (op) == MINUS)
 	      && GET_CODE (XEXP (op, 0)) == MULT)
@@ -4163,8 +4154,7 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return false;
 
     case FMA:
-      if (ISA_HAS_FP_MADDF_MSUBF)
-	*total = mips_fp_mult_cost (mode);
+      *total = mips_fp_mult_cost (mode);
       return false;
 
     case MULT:
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index bceef31..7a6f917 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -236,6 +236,7 @@ struct mips_cpu_info {
 #define TARGET_MIPS5500             (mips_arch == PROCESSOR_R5500)
 #define TARGET_MIPS5900             (mips_arch == PROCESSOR_R5900)
 #define TARGET_MIPS7000             (mips_arch == PROCESSOR_R7000)
+#define TARGET_MIPS8000             (mips_arch == PROCESSOR_R8000)
 #define TARGET_MIPS9000             (mips_arch == PROCESSOR_R9000)
 #define TARGET_OCTEON		    (mips_arch == PROCESSOR_OCTEON	\
 				     || mips_arch == PROCESSOR_OCTEON2	\
@@ -998,22 +999,21 @@ struct mips_cpu_info {
 /* Integer multiply-accumulate instructions should be generated.  */
 #define GENERATE_MADD_MSUB	(TARGET_IMADD && !TARGET_MIPS16)
 
-/* ISA has floating-point madd and msub instructions 'd = a * b [+-] c'.  */
-#define ISA_HAS_FP_MADD4_MSUB4  ISA_HAS_FP4
+/* ISA has 4 operand fused madd instructions of the form
+   'd = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_FUSED_MADD4	TARGET_MIPS8000
 
-/* ISA has floating-point MADDF and MSUBF instructions 'd = d [+-] a * b'.  */
-#define ISA_HAS_FP_MADDF_MSUBF  (mips_isa_rev >= 6)
+/* ISA has 4 operand unfused madd instructions of the form
+   'd = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_UNFUSED_MADD4	(ISA_HAS_FP4 && !TARGET_MIPS8000)
 
-/* ISA has floating-point madd and msub instructions 'c = a * b [+-] c'.  */
-#define ISA_HAS_FP_MADD3_MSUB3  TARGET_LOONGSON_2EF
+/* ISA has 3 operand r6 fused madd instructions of the form
+   'c = c [+-] (a * b)'.  */
+#define ISA_HAS_FUSED_MADDF	(mips_isa_rev >= 6)
 
-/* ISA has floating-point nmadd and nmsub instructions
-   'd = -((a * b) [+-] c)'.  */
-#define ISA_HAS_NMADD4_NMSUB4	ISA_HAS_FP4
-
-/* ISA has floating-point nmadd and nmsub instructions
-   'c = -((a * b) [+-] c)'.  */
-#define ISA_HAS_NMADD3_NMSUB3	TARGET_LOONGSON_2EF
+/* ISA has 3 operand loongson fused madd instructions of the form
+   'c = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_FUSED_MADD3	TARGET_LOONGSON_2EF
 
 /* ISA has floating-point RECIP.fmt and RSQRT.fmt instructions.  The
    MIPS64 rev. 1 ISA says that RECIP.D and RSQRT.D are unpredictable when
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 0a23fa2..4f5692c 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -2475,165 +2475,239 @@
 
 ;; Floating point multiply accumulate instructions.
 
-(define_insn "*madd4<mode>"
+(define_expand "fma<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand")
+		  (match_operand:ANYF 2 "register_operand")
+		  (match_operand:ANYF 3 "register_operand")))]
+  "ISA_HAS_FUSED_MADDF || ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4")
+
+(define_insn "*fma<mode>4_madd3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_FP_MADD4_MSUB4 && TARGET_FUSED_MADD"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "0")))]
+  "ISA_HAS_FUSED_MADD3"
+  "madd.<fmt>\t%0,%1,%2"
+  [(set_attr "type" "fmadd")
+   (set_attr "mode" "<UNITMODE>")])
+
+(define_insn "*fma<mode>4_madd4"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_FUSED_MADD4"
   "madd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "fma<mode>4"
+(define_insn "*fma<mode>4_maddf"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
 		  (match_operand:ANYF 2 "register_operand" "f")
 		  (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADDF_MSUBF"
+  "ISA_HAS_FUSED_MADDF"
   "maddf.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*madd3<mode>"
+;; The fms, fnma, and fnms instructions can be used even when HONOR_NANS
+;; is true because while IEEE 754-2008 requires the negate operation to
+;; negate the sign of a NAN and the MIPS neg instruction does not do this,
+;; the fma part of the instruction has no requirement on how the sign of
+;; a NAN is handled and so the final sign bit of the entire operation is
+;; undefined.
+
+(define_expand "fms<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand")
+		  (match_operand:ANYF 2 "register_operand")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)")
+
+(define_insn "*fms<mode>4_msub3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADD3_MSUB3 && TARGET_FUSED_MADD"
-  "madd.<fmt>\t%0,%1,%2"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand" "0"))))]
+  "ISA_HAS_FUSED_MADD3"
+  "msub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*msub4<mode>"
+(define_insn "*fms<mode>4_msub4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			       (match_operand:ANYF 2 "register_operand" "f"))
-		    (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_FP_MADD4_MSUB4 && TARGET_FUSED_MADD"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_FUSED_MADD4"
   "msub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*msub3<mode>"
+;; fnma is defined in GCC as (fma (neg op1) op2 op3)
+;; (-op1 * op2) + op3 ==> -(op1 * op2) + op3 ==> -((op1 * op2) - op3)
+;; The mips nmsub instructions implement -((op1 * op2) - op3)
+;; This transformation means we may return the wrong signed zero
+;; so we check HONOR_SIGNED_ZEROS.
+
+(define_expand "fnma<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand"))
+		  (match_operand:ANYF 2 "register_operand")
+		  (match_operand:ANYF 3 "register_operand")))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)")
+
+(define_insn "*fnma<mode>4_nmsub3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			       (match_operand:ANYF 2 "register_operand" "f"))
-		    (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADD3_MSUB3 && TARGET_FUSED_MADD"
-  "msub.<fmt>\t%0,%1,%2"
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "0")))]
+  "ISA_HAS_FUSED_MADD3 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd4<mode>"
+(define_insn "*fnma<mode>4_nmsub4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (plus:ANYF
-		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmadd.<fmt>\t%0,%3,%1,%2"
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_FUSED_MADD4 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd3<mode>"
+;; fnms is defined as: (fma (neg op1) op2 (neg op3))
+;; ((-op1) * op2) - op3 ==> -(op1 * op2) - op3 ==> -((op1 * op2) + op3)
+;; The mips nmadd instructions implement -((op1 * op2) + op3)
+;; This transformation means we may return the wrong signed zero
+;; so we check HONOR_SIGNED_ZEROS.
+
+(define_expand "fnms<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand"))
+	  (match_operand:ANYF 2 "register_operand")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)")
+
+(define_insn "*fnms<mode>4_nmadd3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (plus:ANYF
-		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+	  (match_operand:ANYF 2 "register_operand" "f")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand" "0"))))]
+  "ISA_HAS_FUSED_MADD3 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd4<mode>_fastmath"
+(define_insn "*fnms<mode>4_nmadd4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF
-	 (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
-		    (match_operand:ANYF 2 "register_operand" "f"))
-	 (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+	  (match_operand:ANYF 2 "register_operand" "f")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_FUSED_MADD4 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd3<mode>_fastmath"
+;; Non-fused Floating point multiply accumulate instructions.
+
+;; These instructions are not fused and round in between the multiply
+;; and the add (or subtract) so they are equivalent to the separate
+;; multiply and add/sub instructions.
+
+(define_insn "*madd4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF
-	 (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
-		    (match_operand:ANYF 2 "register_operand" "f"))
-	 (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmadd.<fmt>\t%0,%1,%2"
+	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "madd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub4<mode>"
+(define_insn "*msub4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (minus:ANYF
-		   (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-			      (match_operand:ANYF 3 "register_operand" "f"))
-		   (match_operand:ANYF 1 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2,%3"
+	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			       (match_operand:ANYF 2 "register_operand" "f"))
+		    (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "msub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub3<mode>"
+;; Like with the fused fms, fnma, and fnms instructions, these unfused
+;; instructions can be used even if HONOR_NANS is set because while
+;; IEEE 754-2008 requires the negate operation to negate the sign of a
+;; NAN and the MIPS neg instruction does not do this, the multiply and
+;; add (or subtract) part of the instruction has no requirement on how
+;; the sign of a NAN is handled and so the final sign bit of the entire
+;; operation is undefined.
+
+(define_insn "*nmadd4<mode>"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(neg:ANYF (plus:ANYF
+		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "nmadd.<fmt>\t%0,%3,%1,%2"
+  [(set_attr "type" "fmadd")
+   (set_attr "mode" "<UNITMODE>")])
+
+(define_insn "*nmsub4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(neg:ANYF (minus:ANYF
-		   (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-			      (match_operand:ANYF 3 "register_operand" "f"))
-		   (match_operand:ANYF 1 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2"
+		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "nmsub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub4<mode>_fastmath"
+;; Fast-math Non-fused Floating point multiply accumulate instructions.
+
+;; These instructions are not fused but the expressions they match are
+;; not exactly what the instruction implements in the sense that they
+;; may not generate the properly signed zeros.
+
+;; This instruction recognizes  ((-op1) * op2) - op3 and generates an
+;; nmadd which is really -((op1 * op2) + op3).  They are equivalent
+;; except for the sign bit when the result is zero or NaN.
+
+(define_insn "*nmadd4<mode>_fastmath"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(minus:ANYF
-	 (match_operand:ANYF 1 "register_operand" "f")
-	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-		    (match_operand:ANYF 3 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2,%3"
+	  (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		     (match_operand:ANYF 2 "register_operand" "f"))
+	  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub3<mode>_fastmath"
+;; This instruction recognizes (op1 - (op2 * op3) and generates an
+;; nmsub which is really -((op2 * op3) - op1).  They are equivalent
+;; except for the sign bit when the result is zero or NaN.
+
+(define_insn "*nmsub4<mode>_fastmath"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(minus:ANYF
-	 (match_operand:ANYF 1 "register_operand" "f")
-	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-		    (match_operand:ANYF 3 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2"
+	  (match_operand:ANYF 1 "register_operand" "f")
+	  (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
+		     (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index a9baebe..348c6e0 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -209,10 +209,6 @@ mflush-func=
 Target RejectNegative Joined Var(mips_cache_flush_func) Init(CACHE_FLUSH_FUNC)
 -mflush-func=FUNC	Use FUNC to flush the cache before calling stack trampolines
 
-mfused-madd
-Target Report Var(TARGET_FUSED_MADD) Init(1)
-Generate floating-point multiply-add instructions
-
 mabs=
 Target RejectNegative Joined Enum(mips_ieee_754_value) Var(mips_abs) Init(MIPS_IEEE_754_DEFAULT)
 -mabs=MODE	Select the IEEE 754 ABS/NEG instruction execution mode




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 17:45                   ` Steve Ellcey
@ 2015-06-17 18:44                     ` Maciej W. Rozycki
  2015-06-17 18:52                       ` Richard Sandiford
  2015-06-17 19:51                       ` Steve Ellcey
  0 siblings, 2 replies; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-17 18:44 UTC (permalink / raw)
  To: Steve Ellcey
  Cc: Richard Sandiford, Joseph Myers, gcc-patches, Catherine Moore,
	Matthew Fortune

On Wed, 17 Jun 2015, Steve Ellcey wrote:

> Well, I don't mind removing the HONOR_NAN checks from the MIPS code in
> my patch but I am not sure I can do a patch to remove it from the shared
> code.  I see about 80 HONOR_NAN checks in the shared code and I am not
> sure which ones can and cannot be removed.

 FAOD I meant to remove the checks globally throughout MIPS target code 
only.

> Is there any reason why my patch (minus the HONOR_NAN checks) would have
> to wait for the other changes?

 Because it combines two functionally independent changes:

1. HONOR_NAN check removal.

2. FMA support addition.

Worse yet, syntactically overlapping, so that e.g. it's impossible to 
bisect the cause of any possible regression caused without reconstructing 
the two changes from the patch committed if it went in as a single change.

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 18:44                     ` Maciej W. Rozycki
@ 2015-06-17 18:52                       ` Richard Sandiford
  2015-06-17 21:09                         ` Steve Ellcey
  2015-06-18 12:04                         ` Maciej W. Rozycki
  2015-06-17 19:51                       ` Steve Ellcey
  1 sibling, 2 replies; 25+ messages in thread
From: Richard Sandiford @ 2015-06-17 18:52 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Steve Ellcey, Richard Sandiford, Joseph Myers, gcc-patches,
	Catherine Moore, Matthew Fortune

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
> On Wed, 17 Jun 2015, Steve Ellcey wrote:
>> Well, I don't mind removing the HONOR_NAN checks from the MIPS code in
>> my patch but I am not sure I can do a patch to remove it from the shared
>> code.  I see about 80 HONOR_NAN checks in the shared code and I am not
>> sure which ones can and cannot be removed.
>
>  FAOD I meant to remove the checks globally throughout MIPS target code 
> only.

FWIW, to be specific, I think we're talking about every check except
the last two in mips.md:

(define_insn "abs<mode>2"
  [(set (match_operand:ANYF 0 "register_operand" "=f")
	(abs:ANYF (match_operand:ANYF 1 "register_operand" "f")))]
  "mips_abs == MIPS_IEEE_754_2008 || !HONOR_NANS (<MODE>mode)"
  "abs.<fmt>\t%0,%1"
  [(set_attr "type" "fabs")
   (set_attr "mode" "<UNITMODE>")])

(define_insn "neg<mode>2"
  [(set (match_operand:ANYF 0 "register_operand" "=f")
	(neg:ANYF (match_operand:ANYF 1 "register_operand" "f")))]
  "mips_abs == MIPS_IEEE_754_2008 || !HONOR_NANS (<MODE>mode)"
  "neg.<fmt>\t%0,%1"
  [(set_attr "type" "fneg")
   (set_attr "mode" "<UNITMODE>")])

and the one mips-ps-3d.md:

(define_expand "mips_abs_ps"
  [(set (match_operand:V2SF 0 "register_operand")
	(unspec:V2SF [(match_operand:V2SF 1 "register_operand")]
		     UNSPEC_ABS_PS))]
  "TARGET_HARD_FLOAT && 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;
    }
})

In particular, the two checks in mips.c should go.

But like I say, please do add a comment above the unfused patterns to say
why we can match NEG here without the HONOR_NANS test.  It's going to be
far from obvious to anyone who hasn't read this thread.

It's also worth pointing out (although it's probably obvious) that this is
effectively going to be a no-op change.  We'd never have a NEG to combine
if we were honouring NANs and using the legacy pre-2008 mode.  We should
still be accurate though.  Especially when it's also less code :-)

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 18:52                       ` Richard Sandiford
@ 2015-06-17 21:09                         ` Steve Ellcey
  2015-06-17 22:06                           ` Matthew Fortune
  2015-06-18 12:04                         ` Maciej W. Rozycki
  1 sibling, 1 reply; 25+ messages in thread
From: Steve Ellcey @ 2015-06-17 21:09 UTC (permalink / raw)
  To: Richard Sandiford
  Cc: Maciej W. Rozycki, Richard Sandiford, Joseph Myers, gcc-patches,
	Catherine Moore, Matthew Fortune

On Wed, 2015-06-17 at 19:44 +0100, Richard Sandiford wrote:

> 
> FWIW, to be specific, I think we're talking about every check except
> the last two in mips.md:
> 
> and the one mips-ps-3d.md:
> 
> In particular, the two checks in mips.c should go.

OK, here is a patch that does that.

> But like I say, please do add a comment above the unfused patterns to say
> why we can match NEG here without the HONOR_NANS test.  It's going to be
> far from obvious to anyone who hasn't read this thread.

I put these comments in mips.md as part of the patch:

;; The various multiply accumulate instructions can be used even when
;; HONOR_NANS is true because while IEEE 754-2008 requires the negate
;; operation to negate the sign of a NAN and the MIPS neg instruction does
;; not do this, the multiply and add (or minus) parts of these instructions
;; have no requirement on how the sign of a NAN is handled and so the final
;; sign bit of the entire operation is undefined.

> It's also worth pointing out (although it's probably obvious) that this is
> effectively going to be a no-op change.  We'd never have a NEG to combine
> if we were honouring NANs and using the legacy pre-2008 mode.  We should
> still be accurate though.  Especially when it's also less code :-)

Actually, this wasn't obvious to me.  I thought this must have some
affect but I ran through the various options and architectures and, sure
enough, I found no differences in the generated code.

Here is my "prequel" patch.  How does this look?

Steve Ellcey
sellcey@imgtec.com

2015-06-17  Steve Ellcey  <sellcey@imgtec.com>

	* config/mips/mips.c (mips_rtx_costs): Remove HONOR_NAN check.
	* config/mips/mips.md (*madd4<mode>): Ditto.
	(*nmadd3<mode>) Ditto.
	(*nmadd4<mode>_fastmath): Ditto.
	(*nmadd3<mode>_fastmath): Ditto.
	(*nmsub4<mode>): Ditto.
	(*nmsub3<mode>): Ditto.
	(*nmsub4<mode>_fastmath): Ditto.
	(*nmsub3<mode>_fastmath): Ditto.

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index d427c0c..1c837cf 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -4069,7 +4069,6 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       if (float_mode_p
 	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
 	  && TARGET_FUSED_MADD
-	  && !HONOR_NANS (mode)
 	  && !HONOR_SIGNED_ZEROS (mode))
 	{
 	  /* See if we can use NMADD or NMSUB.  See mips.md for the
@@ -4137,7 +4136,6 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       if (float_mode_p
 	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
 	  && TARGET_FUSED_MADD
-	  && !HONOR_NANS (mode)
 	  && HONOR_SIGNED_ZEROS (mode))
 	{
 	  /* See if we can use NMADD or NMSUB.  See mips.md for the
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 0a23fa2..f6912e1 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -2475,6 +2475,13 @@
 
 ;; Floating point multiply accumulate instructions.
 
+;; The various multiply accumulate instructions can be used even when
+;; HONOR_NANS is true because while IEEE 754-2008 requires the negate
+;; operation to negate the sign of a NAN and the MIPS neg instruction does
+;; not do this, the multiply and add (or minus) parts of these instructions
+;; have no requirement on how the sign of a NAN is handled and so the final
+;; sign bit of the entire operation is undefined.
+
 (define_insn "*madd4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
@@ -2533,8 +2540,7 @@
 		   (match_operand:ANYF 3 "register_operand" "f"))))]
   "ISA_HAS_NMADD4_NMSUB4
    && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2547,8 +2553,7 @@
 		   (match_operand:ANYF 3 "register_operand" "0"))))]
   "ISA_HAS_NMADD3_NMSUB3
    && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2561,8 +2566,7 @@
 	 (match_operand:ANYF 3 "register_operand" "f")))]
   "ISA_HAS_NMADD4_NMSUB4
    && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2575,8 +2579,7 @@
 	 (match_operand:ANYF 3 "register_operand" "0")))]
   "ISA_HAS_NMADD3_NMSUB3
    && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2589,8 +2592,7 @@
 		   (match_operand:ANYF 1 "register_operand" "f"))))]
   "ISA_HAS_NMADD4_NMSUB4
    && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2603,8 +2605,7 @@
 		   (match_operand:ANYF 1 "register_operand" "0"))))]
   "ISA_HAS_NMADD3_NMSUB3
    && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmsub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2617,8 +2618,7 @@
 		    (match_operand:ANYF 3 "register_operand" "f"))))]
   "ISA_HAS_NMADD4_NMSUB4
    && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
@@ -2631,8 +2631,7 @@
 		    (match_operand:ANYF 3 "register_operand" "0"))))]
   "ISA_HAS_NMADD3_NMSUB3
    && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)
-   && !HONOR_NANS (<MODE>mode)"
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmsub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])



^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 21:09                         ` Steve Ellcey
@ 2015-06-17 22:06                           ` Matthew Fortune
  2015-06-18 12:32                             ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Matthew Fortune @ 2015-06-17 22:06 UTC (permalink / raw)
  To: Steve Ellcey, Richard Sandiford
  Cc: Maciej W. Rozycki, Richard Sandiford, Joseph Myers, gcc-patches,
	Catherine Moore

Steve Ellcey <Steve.Ellcey@imgtec.com> writes:
> On Wed, 2015-06-17 at 19:44 +0100, Richard Sandiford wrote:
> 
> >
> > FWIW, to be specific, I think we're talking about every check except
> > the last two in mips.md:
> >
> > and the one mips-ps-3d.md:
> >
> > In particular, the two checks in mips.c should go.
> 
> OK, here is a patch that does that.
> 
> > But like I say, please do add a comment above the unfused patterns to say
> > why we can match NEG here without the HONOR_NANS test.  It's going to be
> > far from obvious to anyone who hasn't read this thread.
> 
> I put these comments in mips.md as part of the patch:
> 
> ;; The various multiply accumulate instructions can be used even when
> ;; HONOR_NANS is true because while IEEE 754-2008 requires the negate
> ;; operation to negate the sign of a NAN and the MIPS neg instruction does
> ;; not do this, the multiply and add (or minus) parts of these instructions
> ;; have no requirement on how the sign of a NAN is handled and so the final
> ;; sign bit of the entire operation is undefined.
> 
> > It's also worth pointing out (although it's probably obvious) that this is
> > effectively going to be a no-op change.  We'd never have a NEG to combine
> > if we were honouring NANs and using the legacy pre-2008 mode.  We should
> > still be accurate though.  Especially when it's also less code :-)
> 
> Actually, this wasn't obvious to me.  I thought this must have some
> affect but I ran through the various options and architectures and, sure
> enough, I found no differences in the generated code.
> 
> Here is my "prequel" patch.  How does this look?
> 
> Steve Ellcey
> sellcey@imgtec.com
> 
> 2015-06-17  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	* config/mips/mips.c (mips_rtx_costs): Remove HONOR_NAN check.
> 	* config/mips/mips.md (*madd4<mode>): Ditto.
> 	(*nmadd3<mode>) Ditto.
> 	(*nmadd4<mode>_fastmath): Ditto.
> 	(*nmadd3<mode>_fastmath): Ditto.
> 	(*nmsub4<mode>): Ditto.
> 	(*nmsub3<mode>): Ditto.
> 	(*nmsub4<mode>_fastmath): Ditto.
> 	(*nmsub3<mode>_fastmath): Ditto.

I'd like to delegate to Maciej to approve this and the actual
fp-contract patch. I have nothing of value to add to this
discussion but to say it is clearly comprehensive.

Thanks,
Matthew

> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index d427c0c..1c837cf 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -4069,7 +4069,6 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno
> ATTRIBUTE_UNUSED,
>        if (float_mode_p
>  	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
>  	  && TARGET_FUSED_MADD
> -	  && !HONOR_NANS (mode)
>  	  && !HONOR_SIGNED_ZEROS (mode))
>  	{
>  	  /* See if we can use NMADD or NMSUB.  See mips.md for the
> @@ -4137,7 +4136,6 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno
> ATTRIBUTE_UNUSED,
>        if (float_mode_p
>  	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
>  	  && TARGET_FUSED_MADD
> -	  && !HONOR_NANS (mode)
>  	  && HONOR_SIGNED_ZEROS (mode))
>  	{
>  	  /* See if we can use NMADD or NMSUB.  See mips.md for the
> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
> index 0a23fa2..f6912e1 100644
> --- a/gcc/config/mips/mips.md
> +++ b/gcc/config/mips/mips.md
> @@ -2475,6 +2475,13 @@
> 
>  ;; Floating point multiply accumulate instructions.
> 
> +;; The various multiply accumulate instructions can be used even when
> +;; HONOR_NANS is true because while IEEE 754-2008 requires the negate
> +;; operation to negate the sign of a NAN and the MIPS neg instruction does
> +;; not do this, the multiply and add (or minus) parts of these instructions
> +;; have no requirement on how the sign of a NAN is handled and so the final
> +;; sign bit of the entire operation is undefined.
> +
>  (define_insn "*madd4<mode>"
>    [(set (match_operand:ANYF 0 "register_operand" "=f")
>  	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
> @@ -2533,8 +2540,7 @@
>  		   (match_operand:ANYF 3 "register_operand" "f"))))]
>    "ISA_HAS_NMADD4_NMSUB4
>     && TARGET_FUSED_MADD
> -   && HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmadd.<fmt>\t%0,%3,%1,%2"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2547,8 +2553,7 @@
>  		   (match_operand:ANYF 3 "register_operand" "0"))))]
>    "ISA_HAS_NMADD3_NMSUB3
>     && TARGET_FUSED_MADD
> -   && HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmadd.<fmt>\t%0,%1,%2"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2561,8 +2566,7 @@
>  	 (match_operand:ANYF 3 "register_operand" "f")))]
>    "ISA_HAS_NMADD4_NMSUB4
>     && TARGET_FUSED_MADD
> -   && !HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmadd.<fmt>\t%0,%3,%1,%2"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2575,8 +2579,7 @@
>  	 (match_operand:ANYF 3 "register_operand" "0")))]
>    "ISA_HAS_NMADD3_NMSUB3
>     && TARGET_FUSED_MADD
> -   && !HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmadd.<fmt>\t%0,%1,%2"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2589,8 +2592,7 @@
>  		   (match_operand:ANYF 1 "register_operand" "f"))))]
>    "ISA_HAS_NMADD4_NMSUB4
>     && TARGET_FUSED_MADD
> -   && HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmsub.<fmt>\t%0,%1,%2,%3"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2603,8 +2605,7 @@
>  		   (match_operand:ANYF 1 "register_operand" "0"))))]
>    "ISA_HAS_NMADD3_NMSUB3
>     && TARGET_FUSED_MADD
> -   && HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmsub.<fmt>\t%0,%1,%2"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2617,8 +2618,7 @@
>  		    (match_operand:ANYF 3 "register_operand" "f"))))]
>    "ISA_HAS_NMADD4_NMSUB4
>     && TARGET_FUSED_MADD
> -   && !HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmsub.<fmt>\t%0,%1,%2,%3"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> @@ -2631,8 +2631,7 @@
>  		    (match_operand:ANYF 3 "register_operand" "0"))))]
>    "ISA_HAS_NMADD3_NMSUB3
>     && TARGET_FUSED_MADD
> -   && !HONOR_SIGNED_ZEROS (<MODE>mode)
> -   && !HONOR_NANS (<MODE>mode)"
> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
>    "nmsub.<fmt>\t%0,%1,%2"
>    [(set_attr "type" "fmadd")
>     (set_attr "mode" "<UNITMODE>")])
> 
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 22:06                           ` Matthew Fortune
@ 2015-06-18 12:32                             ` Maciej W. Rozycki
  2015-06-18 16:02                               ` Steve Ellcey
  0 siblings, 1 reply; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-18 12:32 UTC (permalink / raw)
  To: Matthew Fortune
  Cc: Steve Ellcey, Richard Sandiford, Richard Sandiford, Joseph Myers,
	gcc-patches, Catherine Moore

On Wed, 17 Jun 2015, Matthew Fortune wrote:

> > Here is my "prequel" patch.  How does this look?
> > 
> > Steve Ellcey
> > sellcey@imgtec.com
> > 
> > 2015-06-17  Steve Ellcey  <sellcey@imgtec.com>
> > 
> > 	* config/mips/mips.c (mips_rtx_costs): Remove HONOR_NAN check.
> > 	* config/mips/mips.md (*madd4<mode>): Ditto.
> > 	(*nmadd3<mode>) Ditto.
> > 	(*nmadd4<mode>_fastmath): Ditto.
> > 	(*nmadd3<mode>_fastmath): Ditto.
> > 	(*nmsub4<mode>): Ditto.
> > 	(*nmsub3<mode>): Ditto.
> > 	(*nmsub4<mode>_fastmath): Ditto.
> > 	(*nmsub3<mode>_fastmath): Ditto.
> 
> I'd like to delegate to Maciej to approve this and the actual
> fp-contract patch. I have nothing of value to add to this
> discussion but to say it is clearly comprehensive.

 This change looks good to me, I have no objections.  Thanks.

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-18 12:32                             ` Maciej W. Rozycki
@ 2015-06-18 16:02                               ` Steve Ellcey
  2015-06-29 16:10                                 ` Maciej W. Rozycki
  0 siblings, 1 reply; 25+ messages in thread
From: Steve Ellcey @ 2015-06-18 16:02 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Matthew Fortune, Richard Sandiford, Richard Sandiford,
	Joseph Myers, gcc-patches, Catherine Moore

On Thu, 2015-06-18 at 13:04 +0100, Maciej W. Rozycki wrote:

> 
>  This change looks good to me, I have no objections.  Thanks.
> 
>   Maciej

OK, I checked in the prequel patch and here is a new copy of the
original patch based off of that (and with no HONOR_NAN checks in the
fma/madd instructions).

OK for checkin?

Steve Ellcey
sellcey@imgtec.com


2015-06-18  Steve Ellcey  <sellcey@imgtec.com>

	* config.gcc (mips*-*-*): Add fused-madd.opt.
	* config/mips/mips.opt (mfused-madd): Remove.
	* config/mips/mips.c (mips_rtx_costs): Update cost calculations.
	* config/mips/mips.h (TARGET_MIPS8000): New.
	(ISA_HAS_FP_MADD4_MSUB4): Remove.
	(ISA_HAS_FP_MADDF_MSUBF): Remove.
	(ISA_HAS_FP_MADD3_MSUB3): Remove.
	(ISA_HAS_NMADD4_NMSUB4): Remove.
	(ISA_HAS_NMADD3_NMSUB3): Remove.
	(ISA_HAS_FUSED_MADD4): New.
	(ISA_HAS_UNFUSED_MADD4): New.
	(ISA_HAS_FUSED_MADDF): New.
	(ISA_HAS_FUSED_MADD3): New.
	* config/mips/mips.md: (fma<mode>4) Change from insn to expand.
	(*fma<mode>4_madd3) New.
	(*fma<mode>4_madd4) New.
	(*fma<mode>4_maddf) New.
	(fms<mode>4) New.
	(*fms<mode>4_msub3) New.
	(*fms<mode>4_msub4) New.
	(fnma<mode>4) New.
	(*fnma<mode>4_nmadd3) New.
	(*fnma<mode>4_nmadd4) New.
	(fnms<mode>4) New.
	(*fnms<mode>4_nmsub3) New.
	(*fnms<mode>4_nmsub4) New.
	(*madd4<mode>) Modify to be unfused only.
	(*msub4<mode>) Modify to be unfused only.
	(*nmadd4<mode>) Modify to be unfused only.
	(*nmsub4<mode>) Modify to be unfused only.
	(*madd3<mode>) Remove.
	(*msub3<mode>) Remove.
	(*nmadd3<mode>) Remove.
	(*nmsub3<mode>) Remove.
	(*nmadd3<mode>_fastmath) Remove.
	(*nmsub3<mode>_fastmath) Remove.
	(*nmadd4<mode>_fastmath) Update condition.
	(*nmsub4<mode>_fastmath)  Update condition.


diff --git a/gcc/config.gcc b/gcc/config.gcc
index 805638d..fa1dd40 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -418,7 +418,7 @@ microblaze*-*-*)
 mips*-*-*)
 	cpu_type=mips
 	extra_headers="loongson.h"
-	extra_options="${extra_options} g.opt mips/mips-tables.opt"
+	extra_options="${extra_options} g.opt fused-madd.opt mips/mips-tables.opt"
 	;;
 nds32*)
 	cpu_type=nds32
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 1c837cf..df5ab22 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -4066,13 +4066,11 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return true;
 
     case MINUS:
-      if (float_mode_p
-	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
-	  && TARGET_FUSED_MADD
-	  && !HONOR_SIGNED_ZEROS (mode))
+      if (float_mode_p && ISA_HAS_UNFUSED_MADD4 && !HONOR_SIGNED_ZEROS (mode))
 	{
-	  /* See if we can use NMADD or NMSUB.  See mips.md for the
-	     associated patterns.  */
+	  /* See if we can use NMADD or NMSUB via the *nmadd4<mode>_fastmath
+	     or *nmsub4<mode>_fastmath patterns.  These patterns check for
+	     HONOR_SIGNED_ZEROS so we check here too.  */
 	  rtx op0 = XEXP (x, 0);
 	  rtx op1 = XEXP (x, 1);
 	  if (GET_CODE (op0) == MULT && GET_CODE (XEXP (op0, 0)) == NEG)
@@ -4099,9 +4097,7 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
 	{
 	  /* If this is part of a MADD or MSUB, treat the PLUS as
 	     being free.  */
-	  if ((ISA_HAS_FP_MADD4_MSUB4 || ISA_HAS_FP_MADD3_MSUB3)
-	      && TARGET_FUSED_MADD
-	      && GET_CODE (XEXP (x, 0)) == MULT)
+	  if (ISA_HAS_UNFUSED_MADD4 && GET_CODE (XEXP (x, 0)) == MULT)
 	    *total = 0;
 	  else
 	    *total = mips_cost->fp_add;
@@ -4133,13 +4129,10 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return true;
 
     case NEG:
-      if (float_mode_p
-	  && (ISA_HAS_NMADD4_NMSUB4 || ISA_HAS_NMADD3_NMSUB3)
-	  && TARGET_FUSED_MADD
-	  && HONOR_SIGNED_ZEROS (mode))
+      if (float_mode_p && ISA_HAS_UNFUSED_MADD4)
 	{
-	  /* See if we can use NMADD or NMSUB.  See mips.md for the
-	     associated patterns.  */
+	  /* See if we can use NMADD or NMSUB via the *nmadd4<mode> or
+	     *nmsub4<mode> patterns.  */
 	  rtx op = XEXP (x, 0);
 	  if ((GET_CODE (op) == PLUS || GET_CODE (op) == MINUS)
 	      && GET_CODE (XEXP (op, 0)) == MULT)
@@ -4159,8 +4152,7 @@ mips_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       return false;
 
     case FMA:
-      if (ISA_HAS_FP_MADDF_MSUBF)
-	*total = mips_fp_mult_cost (mode);
+      *total = mips_fp_mult_cost (mode);
       return false;
 
     case MULT:
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index bceef31..7a6f917 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -236,6 +236,7 @@ struct mips_cpu_info {
 #define TARGET_MIPS5500             (mips_arch == PROCESSOR_R5500)
 #define TARGET_MIPS5900             (mips_arch == PROCESSOR_R5900)
 #define TARGET_MIPS7000             (mips_arch == PROCESSOR_R7000)
+#define TARGET_MIPS8000             (mips_arch == PROCESSOR_R8000)
 #define TARGET_MIPS9000             (mips_arch == PROCESSOR_R9000)
 #define TARGET_OCTEON		    (mips_arch == PROCESSOR_OCTEON	\
 				     || mips_arch == PROCESSOR_OCTEON2	\
@@ -998,22 +999,21 @@ struct mips_cpu_info {
 /* Integer multiply-accumulate instructions should be generated.  */
 #define GENERATE_MADD_MSUB	(TARGET_IMADD && !TARGET_MIPS16)
 
-/* ISA has floating-point madd and msub instructions 'd = a * b [+-] c'.  */
-#define ISA_HAS_FP_MADD4_MSUB4  ISA_HAS_FP4
+/* ISA has 4 operand fused madd instructions of the form
+   'd = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_FUSED_MADD4	TARGET_MIPS8000
 
-/* ISA has floating-point MADDF and MSUBF instructions 'd = d [+-] a * b'.  */
-#define ISA_HAS_FP_MADDF_MSUBF  (mips_isa_rev >= 6)
+/* ISA has 4 operand unfused madd instructions of the form
+   'd = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_UNFUSED_MADD4	(ISA_HAS_FP4 && !TARGET_MIPS8000)
 
-/* ISA has floating-point madd and msub instructions 'c = a * b [+-] c'.  */
-#define ISA_HAS_FP_MADD3_MSUB3  TARGET_LOONGSON_2EF
+/* ISA has 3 operand r6 fused madd instructions of the form
+   'c = c [+-] (a * b)'.  */
+#define ISA_HAS_FUSED_MADDF	(mips_isa_rev >= 6)
 
-/* ISA has floating-point nmadd and nmsub instructions
-   'd = -((a * b) [+-] c)'.  */
-#define ISA_HAS_NMADD4_NMSUB4	ISA_HAS_FP4
-
-/* ISA has floating-point nmadd and nmsub instructions
-   'c = -((a * b) [+-] c)'.  */
-#define ISA_HAS_NMADD3_NMSUB3	TARGET_LOONGSON_2EF
+/* ISA has 3 operand loongson fused madd instructions of the form
+   'c = [+-] (a * b [+-] c)'.  */
+#define ISA_HAS_FUSED_MADD3	TARGET_LOONGSON_2EF
 
 /* ISA has floating-point RECIP.fmt and RSQRT.fmt instructions.  The
    MIPS64 rev. 1 ISA says that RECIP.D and RSQRT.D are unpredictable when
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index f6912e1..4f5692c 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -2475,164 +2475,239 @@
 
 ;; Floating point multiply accumulate instructions.
 
-;; The various multiply accumulate instructions can be used even when
-;; HONOR_NANS is true because while IEEE 754-2008 requires the negate
-;; operation to negate the sign of a NAN and the MIPS neg instruction does
-;; not do this, the multiply and add (or minus) parts of these instructions
-;; have no requirement on how the sign of a NAN is handled and so the final
-;; sign bit of the entire operation is undefined.
+(define_expand "fma<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand")
+		  (match_operand:ANYF 2 "register_operand")
+		  (match_operand:ANYF 3 "register_operand")))]
+  "ISA_HAS_FUSED_MADDF || ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4")
 
-(define_insn "*madd4<mode>"
+(define_insn "*fma<mode>4_madd3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_FP_MADD4_MSUB4 && TARGET_FUSED_MADD"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "0")))]
+  "ISA_HAS_FUSED_MADD3"
+  "madd.<fmt>\t%0,%1,%2"
+  [(set_attr "type" "fmadd")
+   (set_attr "mode" "<UNITMODE>")])
+
+(define_insn "*fma<mode>4_madd4"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_FUSED_MADD4"
   "madd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "fma<mode>4"
+(define_insn "*fma<mode>4_maddf"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
 		  (match_operand:ANYF 2 "register_operand" "f")
 		  (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADDF_MSUBF"
+  "ISA_HAS_FUSED_MADDF"
   "maddf.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*madd3<mode>"
+;; The fms, fnma, and fnms instructions can be used even when HONOR_NANS
+;; is true because while IEEE 754-2008 requires the negate operation to
+;; negate the sign of a NAN and the MIPS neg instruction does not do this,
+;; the fma part of the instruction has no requirement on how the sign of
+;; a NAN is handled and so the final sign bit of the entire operation is
+;; undefined.
+
+(define_expand "fms<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (match_operand:ANYF 1 "register_operand")
+		  (match_operand:ANYF 2 "register_operand")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)")
+
+(define_insn "*fms<mode>4_msub3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADD3_MSUB3 && TARGET_FUSED_MADD"
-  "madd.<fmt>\t%0,%1,%2"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand" "0"))))]
+  "ISA_HAS_FUSED_MADD3"
+  "msub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*msub4<mode>"
+(define_insn "*fms<mode>4_msub4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			       (match_operand:ANYF 2 "register_operand" "f"))
-		    (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_FP_MADD4_MSUB4 && TARGET_FUSED_MADD"
+	(fma:ANYF (match_operand:ANYF 1 "register_operand" "f")
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (neg:ANYF (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_FUSED_MADD4"
   "msub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*msub3<mode>"
+;; fnma is defined in GCC as (fma (neg op1) op2 op3)
+;; (-op1 * op2) + op3 ==> -(op1 * op2) + op3 ==> -((op1 * op2) - op3)
+;; The mips nmsub instructions implement -((op1 * op2) - op3)
+;; This transformation means we may return the wrong signed zero
+;; so we check HONOR_SIGNED_ZEROS.
+
+(define_expand "fnma<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand"))
+		  (match_operand:ANYF 2 "register_operand")
+		  (match_operand:ANYF 3 "register_operand")))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)")
+
+(define_insn "*fnma<mode>4_nmsub3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			       (match_operand:ANYF 2 "register_operand" "f"))
-		    (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_FP_MADD3_MSUB3 && TARGET_FUSED_MADD"
-  "msub.<fmt>\t%0,%1,%2"
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "0")))]
+  "ISA_HAS_FUSED_MADD3 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd4<mode>"
+(define_insn "*fnma<mode>4_nmsub4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (plus:ANYF
-		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)"
-  "nmadd.<fmt>\t%0,%3,%1,%2"
+	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		  (match_operand:ANYF 2 "register_operand" "f")
+		  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_FUSED_MADD4 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+  "nmsub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd3<mode>"
+;; fnms is defined as: (fma (neg op1) op2 (neg op3))
+;; ((-op1) * op2) - op3 ==> -(op1 * op2) - op3 ==> -((op1 * op2) + op3)
+;; The mips nmadd instructions implement -((op1 * op2) + op3)
+;; This transformation means we may return the wrong signed zero
+;; so we check HONOR_SIGNED_ZEROS.
+
+(define_expand "fnms<mode>4"
+  [(set (match_operand:ANYF 0 "register_operand")
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand"))
+	  (match_operand:ANYF 2 "register_operand")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
+  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
+   && !HONOR_SIGNED_ZEROS (<MODE>mode)")
+
+(define_insn "*fnms<mode>4_nmadd3"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (plus:ANYF
-		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
-			      (match_operand:ANYF 2 "register_operand" "f"))
-		   (match_operand:ANYF 3 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)"
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+	  (match_operand:ANYF 2 "register_operand" "f")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand" "0"))))]
+  "ISA_HAS_FUSED_MADD3 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd4<mode>_fastmath"
+(define_insn "*fnms<mode>4_nmadd4"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF
-	 (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
-		    (match_operand:ANYF 2 "register_operand" "f"))
-	 (match_operand:ANYF 3 "register_operand" "f")))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
+	(fma:ANYF
+	  (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+	  (match_operand:ANYF 2 "register_operand" "f")
+	  (neg:ANYF (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_FUSED_MADD4 && !HONOR_SIGNED_ZEROS (<MODE>mode)"
   "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmadd3<mode>_fastmath"
+;; Non-fused Floating point multiply accumulate instructions.
+
+;; These instructions are not fused and round in between the multiply
+;; and the add (or subtract) so they are equivalent to the separate
+;; multiply and add/sub instructions.
+
+(define_insn "*madd4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(minus:ANYF
-	 (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
-		    (match_operand:ANYF 2 "register_operand" "f"))
-	 (match_operand:ANYF 3 "register_operand" "0")))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && !HONOR_SIGNED_ZEROS (<MODE>mode)"
-  "nmadd.<fmt>\t%0,%1,%2"
+	(plus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "madd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub4<mode>"
+(define_insn "*msub4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
-	(neg:ANYF (minus:ANYF
-		   (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-			      (match_operand:ANYF 3 "register_operand" "f"))
-		   (match_operand:ANYF 1 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2,%3"
+	(minus:ANYF (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			       (match_operand:ANYF 2 "register_operand" "f"))
+		    (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "msub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub3<mode>"
+;; Like with the fused fms, fnma, and fnms instructions, these unfused
+;; instructions can be used even if HONOR_NANS is set because while
+;; IEEE 754-2008 requires the negate operation to negate the sign of a
+;; NAN and the MIPS neg instruction does not do this, the multiply and
+;; add (or subtract) part of the instruction has no requirement on how
+;; the sign of a NAN is handled and so the final sign bit of the entire
+;; operation is undefined.
+
+(define_insn "*nmadd4<mode>"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(neg:ANYF (plus:ANYF
+		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "nmadd.<fmt>\t%0,%3,%1,%2"
+  [(set_attr "type" "fmadd")
+   (set_attr "mode" "<UNITMODE>")])
+
+(define_insn "*nmsub4<mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(neg:ANYF (minus:ANYF
-		   (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-			      (match_operand:ANYF 3 "register_operand" "f"))
-		   (match_operand:ANYF 1 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
-   && HONOR_SIGNED_ZEROS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2"
+		   (mult:ANYF (match_operand:ANYF 1 "register_operand" "f")
+			      (match_operand:ANYF 2 "register_operand" "f"))
+		   (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4"
+  "nmsub.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub4<mode>_fastmath"
+;; Fast-math Non-fused Floating point multiply accumulate instructions.
+
+;; These instructions are not fused but the expressions they match are
+;; not exactly what the instruction implements in the sense that they
+;; may not generate the properly signed zeros.
+
+;; This instruction recognizes  ((-op1) * op2) - op3 and generates an
+;; nmadd which is really -((op1 * op2) + op3).  They are equivalent
+;; except for the sign bit when the result is zero or NaN.
+
+(define_insn "*nmadd4<mode>_fastmath"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(minus:ANYF
-	 (match_operand:ANYF 1 "register_operand" "f")
-	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-		    (match_operand:ANYF 3 "register_operand" "f"))))]
-  "ISA_HAS_NMADD4_NMSUB4
-   && TARGET_FUSED_MADD
+	  (mult:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand" "f"))
+		     (match_operand:ANYF 2 "register_operand" "f"))
+	  (match_operand:ANYF 3 "register_operand" "f")))]
+  "ISA_HAS_UNFUSED_MADD4
    && !HONOR_SIGNED_ZEROS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2,%3"
+  "nmadd.<fmt>\t%0,%3,%1,%2"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
-(define_insn "*nmsub3<mode>_fastmath"
+;; This instruction recognizes (op1 - (op2 * op3) and generates an
+;; nmsub which is really -((op2 * op3) - op1).  They are equivalent
+;; except for the sign bit when the result is zero or NaN.
+
+(define_insn "*nmsub4<mode>_fastmath"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(minus:ANYF
-	 (match_operand:ANYF 1 "register_operand" "f")
-	 (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
-		    (match_operand:ANYF 3 "register_operand" "0"))))]
-  "ISA_HAS_NMADD3_NMSUB3
-   && TARGET_FUSED_MADD
+	  (match_operand:ANYF 1 "register_operand" "f")
+	  (mult:ANYF (match_operand:ANYF 2 "register_operand" "f")
+		     (match_operand:ANYF 3 "register_operand" "f"))))]
+  "ISA_HAS_UNFUSED_MADD4
    && !HONOR_SIGNED_ZEROS (<MODE>mode)"
-  "nmsub.<fmt>\t%0,%1,%2"
+  "nmsub.<fmt>\t%0,%1,%2,%3"
   [(set_attr "type" "fmadd")
    (set_attr "mode" "<UNITMODE>")])
 
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index a9baebe..348c6e0 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -209,10 +209,6 @@ mflush-func=
 Target RejectNegative Joined Var(mips_cache_flush_func) Init(CACHE_FLUSH_FUNC)
 -mflush-func=FUNC	Use FUNC to flush the cache before calling stack trampolines
 
-mfused-madd
-Target Report Var(TARGET_FUSED_MADD) Init(1)
-Generate floating-point multiply-add instructions
-
 mabs=
 Target RejectNegative Joined Enum(mips_ieee_754_value) Var(mips_abs) Init(MIPS_IEEE_754_DEFAULT)
 -mabs=MODE	Select the IEEE 754 ABS/NEG instruction execution mode


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-18 16:02                               ` Steve Ellcey
@ 2015-06-29 16:10                                 ` Maciej W. Rozycki
  2015-06-29 19:07                                   ` Matthew Fortune
  2015-06-30  7:35                                   ` Richard Biener
  0 siblings, 2 replies; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-29 16:10 UTC (permalink / raw)
  To: Steve Ellcey, Richard Biener
  Cc: Matthew Fortune, Richard Sandiford, Richard Sandiford,
	Joseph Myers, gcc-patches, Catherine Moore

Richard, please have a look at my question below in a reference to your 
previous statement.

On Thu, 18 Jun 2015, Steve Ellcey wrote:

> OK, I checked in the prequel patch and here is a new copy of the
> original patch based off of that (and with no HONOR_NAN checks in the
> fma/madd instructions).
> 
> OK for checkin?

 Please see below for my notes.

> 2015-06-18  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	* config.gcc (mips*-*-*): Add fused-madd.opt.

 Please use angle brackets as per 
<https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-Changed.html>, 
i.e.:

	* config.gcc <mips*-*-*>: Add fused-madd.opt.

There's no function or similar entity involved here and `mips*-*-*' is a 
case value like with the C language's `switch' statement where you'd use 
angle brackets too to refer to individual cases.

> 	(*nmsub4<mode>_fastmath)  Update condition.

 Extraneous space here.

> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
> index f6912e1..4f5692c 100644
> --- a/gcc/config/mips/mips.md
> +++ b/gcc/config/mips/mips.md
[...]
> +;; fnma is defined in GCC as (fma (neg op1) op2 op3)
> +;; (-op1 * op2) + op3 ==> -(op1 * op2) + op3 ==> -((op1 * op2) - op3)
> +;; The mips nmsub instructions implement -((op1 * op2) - op3)
> +;; This transformation means we may return the wrong signed zero
> +;; so we check HONOR_SIGNED_ZEROS.
> +
> +(define_expand "fnma<mode>4"
> +  [(set (match_operand:ANYF 0 "register_operand")
> +	(fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand"))
> +		  (match_operand:ANYF 2 "register_operand")
> +		  (match_operand:ANYF 3 "register_operand")))]
> +  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)")

 Have you considered the alternative/complementary approach proposed by 
Richard here: <http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00680.html>, 
i.e. to introduce further expanders, e.g.:

fmanM4: (neg:M (fma:M OP1 OP2 OP3))		(multiply-add, negated)

fmsnM4: (neg:M (fma:M OP1 OP2 (neg:M OP3)))	(multiply-subtract, negated)

?

 These patterns wouldn't need a check for !HONOR_SIGNED_ZEROS as they 
match the respective hardware instructions in an exact manner.  Therefore 
I think they would be more useful as they would also suit software that 
claims/requires full IEEE Std 754 compliance.

 Richard, do you maintain the introduction of these additional operations
would be a good idea and one you're willing to support for the purpose of 
patch acceptance/approval if implemented?

> +;; fnms is defined as: (fma (neg op1) op2 (neg op3))
> +;; ((-op1) * op2) - op3 ==> -(op1 * op2) - op3 ==> -((op1 * op2) + op3)
> +;; The mips nmadd instructions implement -((op1 * op2) + op3)
> +;; This transformation means we may return the wrong signed zero
> +;; so we check HONOR_SIGNED_ZEROS.
> +
> +(define_expand "fnms<mode>4"
> +  [(set (match_operand:ANYF 0 "register_operand")
> +	(fma:ANYF
> +	  (neg:ANYF (match_operand:ANYF 1 "register_operand"))
> +	  (match_operand:ANYF 2 "register_operand")
> +	  (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
> +  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)")

 Same observation here.

 The change looks good to me otherwise.

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-29 16:10                                 ` Maciej W. Rozycki
@ 2015-06-29 19:07                                   ` Matthew Fortune
  2015-06-29 19:46                                     ` Moore, Catherine
  2015-07-06 17:35                                     ` Steve Ellcey
  2015-06-30  7:35                                   ` Richard Biener
  1 sibling, 2 replies; 25+ messages in thread
From: Matthew Fortune @ 2015-06-29 19:07 UTC (permalink / raw)
  To: Maciej W. Rozycki, Steve Ellcey, Catherine Moore, Richard Biener
  Cc: Richard Sandiford, Richard Sandiford, Joseph Myers, gcc-patches

Maciej W. Rozycki <macro@linux-mips.org> writes:
> Richard, please have a look at my question below in a reference to your
> previous statement.
> 
> On Thu, 18 Jun 2015, Steve Ellcey wrote:
> 
> > OK, I checked in the prequel patch and here is a new copy of the
> > original patch based off of that (and with no HONOR_NAN checks in the
> > fma/madd instructions).
> >
> > OK for checkin?
> 
>  Please see below for my notes.
> 
> > 2015-06-18  Steve Ellcey  <sellcey@imgtec.com>
> >
> > 	* config.gcc (mips*-*-*): Add fused-madd.opt.
> 
>  Please use angle brackets as per
> <https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-Changed.html>,
> i.e.:
> 
> 	* config.gcc <mips*-*-*>: Add fused-madd.opt.
> 
> There's no function or similar entity involved here and `mips*-*-*' is a
> case value like with the C language's `switch' statement where you'd use
> angle brackets too to refer to individual cases.
> 
> > 	(*nmsub4<mode>_fastmath)  Update condition.
> 
>  Extraneous space here.
> 
>  The change looks good to me otherwise.
> 
>   Maciej

If nobody can identify any further functional issues with this patch then
I'd like to get this committed and pursue enhancements as a second round.
Catherine, would you be happy for this to be committed on that basis?

Thanks,
Matthew

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-29 19:07                                   ` Matthew Fortune
@ 2015-06-29 19:46                                     ` Moore, Catherine
  2015-07-06 17:35                                     ` Steve Ellcey
  1 sibling, 0 replies; 25+ messages in thread
From: Moore, Catherine @ 2015-06-29 19:46 UTC (permalink / raw)
  To: Matthew Fortune, Maciej W. Rozycki, Steve Ellcey, Richard Biener
  Cc: Richard Sandiford, Richard Sandiford, Myers, Joseph, gcc-patches



> -----Original Message-----
> From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com]
> Sent: Monday, June 29, 2015 3:07 PM
> To: Maciej W. Rozycki; Steve Ellcey; Moore, Catherine; Richard Biener
> Cc: Richard Sandiford; Richard Sandiford; Myers, Joseph; gcc-
> patches@gcc.gnu.org
> Subject: RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-
> madd
> 
> Maciej W. Rozycki <macro@linux-mips.org> writes:
> > Richard, please have a look at my question below in a reference to
> > your previous statement.
> >
> > On Thu, 18 Jun 2015, Steve Ellcey wrote:
> >
> > > OK, I checked in the prequel patch and here is a new copy of the
> > > original patch based off of that (and with no HONOR_NAN checks in
> > > the fma/madd instructions).
> > >
> > > OK for checkin?
> >
> >  Please see below for my notes.
> >
> > > 2015-06-18  Steve Ellcey  <sellcey@imgtec.com>
> > >
> > > 	* config.gcc (mips*-*-*): Add fused-madd.opt.
> >
> >  Please use angle brackets as per
> > <https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-
> Chan
> > ged.html>,
> > i.e.:
> >
> > 	* config.gcc <mips*-*-*>: Add fused-madd.opt.
> >
> > There's no function or similar entity involved here and `mips*-*-*' is
> > a case value like with the C language's `switch' statement where you'd
> > use angle brackets too to refer to individual cases.
> >
> > > 	(*nmsub4<mode>_fastmath)  Update condition.
> >
> >  Extraneous space here.
> >
> >  The change looks good to me otherwise.
> >
> >   Maciej
> 
> If nobody can identify any further functional issues with this patch then I'd
> like to get this committed and pursue enhancements as a second round.
> Catherine, would you be happy for this to be committed on that basis?
> 
Yes, this is fine with me.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-29 19:07                                   ` Matthew Fortune
  2015-06-29 19:46                                     ` Moore, Catherine
@ 2015-07-06 17:35                                     ` Steve Ellcey
  1 sibling, 0 replies; 25+ messages in thread
From: Steve Ellcey @ 2015-07-06 17:35 UTC (permalink / raw)
  To: Matthew Fortune
  Cc: Maciej W. Rozycki, Catherine Moore, Richard Biener,
	Richard Sandiford, Richard Sandiford, Joseph Myers, gcc-patches

On Mon, 2015-06-29 at 12:07 -0700, Matthew Fortune wrote:

> If nobody can identify any further functional issues with this patch then
> I'd like to get this committed and pursue enhancements as a second round.
> Catherine, would you be happy for this to be committed on that basis?
> 
> Thanks,
> Matthew

Sorry for the delay (I was on vacation), I fixed up the ChangeLog issues
Maciej pointed out and checked this patch in.

Steve Ellcey
sellcey@imgtec.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-29 16:10                                 ` Maciej W. Rozycki
  2015-06-29 19:07                                   ` Matthew Fortune
@ 2015-06-30  7:35                                   ` Richard Biener
  1 sibling, 0 replies; 25+ messages in thread
From: Richard Biener @ 2015-06-30  7:35 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Steve Ellcey, Matthew Fortune, Richard Sandiford,
	Richard Sandiford, Joseph Myers, gcc-patches, Catherine Moore

On Mon, Jun 29, 2015 at 6:08 PM, Maciej W. Rozycki <macro@linux-mips.org> wrote:
> Richard, please have a look at my question below in a reference to your
> previous statement.
>
> On Thu, 18 Jun 2015, Steve Ellcey wrote:
>
>> OK, I checked in the prequel patch and here is a new copy of the
>> original patch based off of that (and with no HONOR_NAN checks in the
>> fma/madd instructions).
>>
>> OK for checkin?
>
>  Please see below for my notes.
>
>> 2015-06-18  Steve Ellcey  <sellcey@imgtec.com>
>>
>>       * config.gcc (mips*-*-*): Add fused-madd.opt.
>
>  Please use angle brackets as per
> <https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-Changed.html>,
> i.e.:
>
>         * config.gcc <mips*-*-*>: Add fused-madd.opt.
>
> There's no function or similar entity involved here and `mips*-*-*' is a
> case value like with the C language's `switch' statement where you'd use
> angle brackets too to refer to individual cases.
>
>>       (*nmsub4<mode>_fastmath)  Update condition.
>
>  Extraneous space here.
>
>> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
>> index f6912e1..4f5692c 100644
>> --- a/gcc/config/mips/mips.md
>> +++ b/gcc/config/mips/mips.md
> [...]
>> +;; fnma is defined in GCC as (fma (neg op1) op2 op3)
>> +;; (-op1 * op2) + op3 ==> -(op1 * op2) + op3 ==> -((op1 * op2) - op3)
>> +;; The mips nmsub instructions implement -((op1 * op2) - op3)
>> +;; This transformation means we may return the wrong signed zero
>> +;; so we check HONOR_SIGNED_ZEROS.
>> +
>> +(define_expand "fnma<mode>4"
>> +  [(set (match_operand:ANYF 0 "register_operand")
>> +     (fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand"))
>> +               (match_operand:ANYF 2 "register_operand")
>> +               (match_operand:ANYF 3 "register_operand")))]
>> +  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
>> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)")
>
>  Have you considered the alternative/complementary approach proposed by
> Richard here: <http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00680.html>,
> i.e. to introduce further expanders, e.g.:
>
> fmanM4: (neg:M (fma:M OP1 OP2 OP3))             (multiply-add, negated)
>
> fmsnM4: (neg:M (fma:M OP1 OP2 (neg:M OP3)))     (multiply-subtract, negated)
>
> ?
>
>  These patterns wouldn't need a check for !HONOR_SIGNED_ZEROS as they
> match the respective hardware instructions in an exact manner.  Therefore
> I think they would be more useful as they would also suit software that
> claims/requires full IEEE Std 754 compliance.
>
>  Richard, do you maintain the introduction of these additional operations
> would be a good idea and one you're willing to support for the purpose of
> patch acceptance/approval if implemented?

Yes, esp. if there is now a second architecture that has such instructions.

Thanks,
Richard.

>> +;; fnms is defined as: (fma (neg op1) op2 (neg op3))
>> +;; ((-op1) * op2) - op3 ==> -(op1 * op2) - op3 ==> -((op1 * op2) + op3)
>> +;; The mips nmadd instructions implement -((op1 * op2) + op3)
>> +;; This transformation means we may return the wrong signed zero
>> +;; so we check HONOR_SIGNED_ZEROS.
>> +
>> +(define_expand "fnms<mode>4"
>> +  [(set (match_operand:ANYF 0 "register_operand")
>> +     (fma:ANYF
>> +       (neg:ANYF (match_operand:ANYF 1 "register_operand"))
>> +       (match_operand:ANYF 2 "register_operand")
>> +       (neg:ANYF (match_operand:ANYF 3 "register_operand"))))]
>> +  "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4)
>> +   && !HONOR_SIGNED_ZEROS (<MODE>mode)")
>
>  Same observation here.
>
>  The change looks good to me otherwise.
>
>   Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 18:52                       ` Richard Sandiford
  2015-06-17 21:09                         ` Steve Ellcey
@ 2015-06-18 12:04                         ` Maciej W. Rozycki
  1 sibling, 0 replies; 25+ messages in thread
From: Maciej W. Rozycki @ 2015-06-18 12:04 UTC (permalink / raw)
  To: Richard Sandiford
  Cc: Steve Ellcey, Richard Sandiford, Joseph Myers, gcc-patches,
	Catherine Moore, Matthew Fortune

On Wed, 17 Jun 2015, Richard Sandiford wrote:

> and the one mips-ps-3d.md:
> 
> (define_expand "mips_abs_ps"
>   [(set (match_operand:V2SF 0 "register_operand")
> 	(unspec:V2SF [(match_operand:V2SF 1 "register_operand")]
> 		     UNSPEC_ABS_PS))]
>   "TARGET_HARD_FLOAT && 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;
>     }
> })

 Hmm, I wonder whether this one shouldn't really be:

   if (mips_abs == MIPS_IEEE_754_2008 || !HONOR_NANS (V2SFmode))

as even though not explicitly specified for an ABS2008 implementation 
that also supports the MIPS-3D ASE, I'd say ABS.PS is supposed to be 
consistent with ABS.D and ABS.S and respect FCSR.ABS2008.

  Maciej

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd
  2015-06-17 18:44                     ` Maciej W. Rozycki
  2015-06-17 18:52                       ` Richard Sandiford
@ 2015-06-17 19:51                       ` Steve Ellcey
  1 sibling, 0 replies; 25+ messages in thread
From: Steve Ellcey @ 2015-06-17 19:51 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Richard Sandiford, Joseph Myers, gcc-patches, Catherine Moore,
	Matthew Fortune

On Wed, 2015-06-17 at 19:17 +0100, Maciej W. Rozycki wrote:
> On Wed, 17 Jun 2015, Steve Ellcey wrote:
> 
>  FAOD I meant to remove the checks globally throughout MIPS target code 
> only.
> 
> > Is there any reason why my patch (minus the HONOR_NAN checks) would have
> > to wait for the other changes?
> 
>  Because it combines two functionally independent changes:
> 
> 1. HONOR_NAN check removal.
> 
> 2. FMA support addition.
> 
> Worse yet, syntactically overlapping, so that e.g. it's impossible to 
> bisect the cause of any possible regression caused without reconstructing 
> the two changes from the patch committed if it went in as a single change.
> 
>   Maciej

OK, that makes more sense.  I misunderstood what you were originally
saying.

Steve Ellcey
sellcey@imgtec.com


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-07-06 17:35 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-11 20:09 [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd Steve Ellcey 
2015-06-11 20:39 ` Joseph Myers
2015-06-15 20:52   ` Maciej W. Rozycki
2015-06-15 21:37     ` Joseph Myers
2015-06-15 22:00       ` Maciej W. Rozycki
2015-06-15 22:09         ` Joseph Myers
2015-06-16 12:19           ` Maciej W. Rozycki
2015-06-16 12:26             ` Joseph Myers
2015-06-16 13:28               ` Maciej W. Rozycki
2015-06-16 16:13                 ` Joseph Myers
2015-06-17 10:43                 ` Richard Sandiford
2015-06-17 17:45                   ` Steve Ellcey
2015-06-17 18:44                     ` Maciej W. Rozycki
2015-06-17 18:52                       ` Richard Sandiford
2015-06-17 21:09                         ` Steve Ellcey
2015-06-17 22:06                           ` Matthew Fortune
2015-06-18 12:32                             ` Maciej W. Rozycki
2015-06-18 16:02                               ` Steve Ellcey
2015-06-29 16:10                                 ` Maciej W. Rozycki
2015-06-29 19:07                                   ` Matthew Fortune
2015-06-29 19:46                                     ` Moore, Catherine
2015-07-06 17:35                                     ` Steve Ellcey
2015-06-30  7:35                                   ` Richard Biener
2015-06-18 12:04                         ` Maciej W. Rozycki
2015-06-17 19:51                       ` Steve Ellcey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).