diff mbox

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

Message ID 1434574154.18552.15.camel@ubuntu-sellcey
State New
Headers show

Commit Message

Steve Ellcey June 17, 2015, 8:49 p.m. UTC 
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.

Comments

Matthew Fortune June 17, 2015, 9:09 p.m. UTC | #1 
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>")])
> 
>
Maciej W. Rozycki June 18, 2015, 12:04 p.m. UTC | #2 
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
diff mbox

Patch

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>")])