diff mbox

MIPS: MIPS32r2 FP reciprocal instruction set support

Message ID alpine.DEB.1.10.1311201657560.21686@tp.orcam.me.uk
State Accepted
Headers show

Commit Message

Maciej W. Rozycki Nov. 20, 2013, 5:19 p.m. UTC 
On Sat, 16 Nov 2013, Richard Sandiford wrote:

> > Index: gcc-fsf-trunk-quilt/gcc/config/mips/mips.h
> > ===================================================================
> > --- gcc-fsf-trunk-quilt.orig/gcc/config/mips/mips.h	2013-11-12 15:31:46.758734464 +0000
> > +++ gcc-fsf-trunk-quilt/gcc/config/mips/mips.h	2013-11-12 15:33:22.277646941 +0000
> > @@ -921,6 +921,21 @@ struct mips_cpu_info {
> >     'c = -((a * b) [+-] c)'.  */
> >  #define ISA_HAS_NMADD3_NMSUB3	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
> > +   doubles are stored in pairs of FPRs, so for safety's sake, we apply
> > +   this restriction to the MIPS IV ISA too.  */
> > +#define ISA_HAS_FP_RECIP_RSQRT(MODE)					\
> > +				(((ISA_HAS_FP4				\
> > +				   || (ISA_MIPS32R2 && !TARGET_MIPS16))	\
> > +				  && ((MODE) == SFmode			\
> > +				      || ((TARGET_FLOAT64		\
> > +					   || !(ISA_MIPS4		\
> > +						|| ISA_MIPS64))		\
> > +					  && (MODE) == DFmode)))	\
> > +				 || ((TARGET_SB1 && !TARGET_MIPS16)	\
> > +				     && (MODE) == V2SFmode))
> 
> I think the !(ISA_MIPS4 || ISA_MIPS64) part is really "r2 or later",
> which elsewhere we test as "ISA_MIPS32R2 || ISA_MIPS64R2".  Obviously
> that isn't as future-proof, but I think consistency wins here.
> (E.g. the earlier ISA_MIPS32R2 seems like it's reallly "r2 or later" too).
> 
> Cleaning up these macros has been on my todo list for about ten years :-(

 Code follows the comment (or vice versa) to make it easier for the reader 
to understand what's going on here, however if you plan to revamp these 
macros, then I'm leaving it up to you.

> Please also test !TARGET_MIPS16 at the outermost level, so that there's
> only one instance.  I think that gives something like:
> 
> #define ISA_HAS_FP_RECIP_RSQRT(MODE)					\
> 				((((ISA_HAS_FP4 || ISA_MIPS32R2)	\
> 				   && ((MODE) == SFmode			\
> 				       || ((TARGET_FLOAT64		\
> 					    || ISA_MIPS32R2		\
> 					    || ISA_MIPS64R2)		\
> 					   && (MODE) == DFmode)))	\
> 				  || (TARGET_SB1			\
> 				      && (MODE) == V2SFmode))		\
> 				 && !TARGET_MIPS16)

 This will make TARGET_MIPS16 checked twice in some cases as ISA_HAS_FP4 
already includes it, but again given your plan to rework all of this I 
have made the adjustment you requested.

> OK with those changes, thanks.

 This is the final change I have applied.  I have verified that it still 
makes the desired effect and successfully retested it for mips-linux-gnu, 
o32 ABI.  Thanks for your review.

2013-11-20  Maciej W. Rozycki  <macro@codesourcery.com>

	gcc/
        * config/mips/mips.h (ISA_HAS_FP_RECIP_RSQRT): New macro.
        * config/mips/mips.c (mips_rtx_costs) <DIV>: Check for
        ISA_HAS_FP_RECIP_RSQRT rather than ISA_HAS_FP4.
        * config/mips/mips.md (recip_condition): Remove mode attribute.
        (div<mode>3): Use ISA_HAS_FP_RECIP_RSQRT rather than
        <recip_condition>.
        (*recip<mode>3, *rsqrt<mode>a, *rsqrt<mode>b): Likewise.

  Maciej

gcc-mips32r2-recip.patch

Comments

Richard Sandiford Nov. 20, 2013, 6:08 p.m. UTC | #1 
"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Sat, 16 Nov 2013, Richard Sandiford wrote:
>
>> > Index: gcc-fsf-trunk-quilt/gcc/config/mips/mips.h
>> > ===================================================================
>> > --- gcc-fsf-trunk-quilt.orig/gcc/config/mips/mips.h	2013-11-12 15:31:46.758734464 +0000
>> > +++ gcc-fsf-trunk-quilt/gcc/config/mips/mips.h	2013-11-12 15:33:22.277646941 +0000
>> > @@ -921,6 +921,21 @@ struct mips_cpu_info {
>> >     'c = -((a * b) [+-] c)'.  */
>> >  #define ISA_HAS_NMADD3_NMSUB3	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
>> > +   doubles are stored in pairs of FPRs, so for safety's sake, we apply
>> > +   this restriction to the MIPS IV ISA too.  */
>> > +#define ISA_HAS_FP_RECIP_RSQRT(MODE)					\
>> > +				(((ISA_HAS_FP4				\
>> > +				   || (ISA_MIPS32R2 && !TARGET_MIPS16))	\
>> > +				  && ((MODE) == SFmode			\
>> > +				      || ((TARGET_FLOAT64		\
>> > +					   || !(ISA_MIPS4		\
>> > +						|| ISA_MIPS64))		\
>> > +					  && (MODE) == DFmode)))	\
>> > +				 || ((TARGET_SB1 && !TARGET_MIPS16)	\
>> > +				     && (MODE) == V2SFmode))
>> 
>> I think the !(ISA_MIPS4 || ISA_MIPS64) part is really "r2 or later",
>> which elsewhere we test as "ISA_MIPS32R2 || ISA_MIPS64R2".  Obviously
>> that isn't as future-proof, but I think consistency wins here.
>> (E.g. the earlier ISA_MIPS32R2 seems like it's reallly "r2 or later" too).
>> 
>> Cleaning up these macros has been on my todo list for about ten years :-(
>
>  Code follows the comment (or vice versa) to make it easier for the reader 
> to understand what's going on here, however if you plan to revamp these 
> macros, then I'm leaving it up to you.

Ah, sorry, I hadn't meant it that way.  I'm happy with the split of
ISA_HAS_* macros into features, including those you're making here.

The cleanup I meant was to replace the long lists of ISA_MIPS* tests
with something a bit shorter and more future-proof.  E.g. something
similar to the __mips and __mips_isa_rev that we provide for users.

Thanks,
Richard
diff mbox

Patch

Index: gcc-fsf-trunk-quilt/gcc/config/mips/mips.c
===================================================================
--- gcc-fsf-trunk-quilt.orig/gcc/config/mips/mips.c	2013-11-19 01:48:08.000000000 +0000
+++ gcc-fsf-trunk-quilt/gcc/config/mips/mips.c	2013-11-19 01:48:25.787735722 +0000
@@ -3968,7 +3968,7 @@  mips_rtx_costs (rtx x, int code, int out
     case DIV:
       /* Check for a reciprocal.  */
       if (float_mode_p
-	  && ISA_HAS_FP4
+	  && ISA_HAS_FP_RECIP_RSQRT (mode)
 	  && flag_unsafe_math_optimizations
 	  && XEXP (x, 0) == CONST1_RTX (mode))
 	{
Index: gcc-fsf-trunk-quilt/gcc/config/mips/mips.h
===================================================================
--- gcc-fsf-trunk-quilt.orig/gcc/config/mips/mips.h	2013-11-19 01:48:08.000000000 +0000
+++ gcc-fsf-trunk-quilt/gcc/config/mips/mips.h	2013-11-19 02:01:22.977885328 +0000
@@ -921,6 +921,21 @@  struct mips_cpu_info {
    'c = -((a * b) [+-] c)'.  */
 #define ISA_HAS_NMADD3_NMSUB3	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
+   doubles are stored in pairs of FPRs, so for safety's sake, we apply
+   this restriction to the MIPS IV ISA too.  */
+#define ISA_HAS_FP_RECIP_RSQRT(MODE)					\
+				((((ISA_HAS_FP4 || ISA_MIPS32R2)	\
+				   && ((MODE) == SFmode			\
+				       || ((TARGET_FLOAT64		\
+					    || ISA_MIPS32R2		\
+					    || ISA_MIPS64R2)		\
+					   && (MODE) == DFmode)))	\
+				  || (TARGET_SB1			\
+				      && (MODE) == V2SFmode))		\
+				 && !TARGET_MIPS16)
+
 /* ISA has count leading zeroes/ones instruction (not implemented).  */
 #define ISA_HAS_CLZ_CLO		((ISA_MIPS32				\
 				  || ISA_MIPS32R2			\
Index: gcc-fsf-trunk-quilt/gcc/config/mips/mips.md
===================================================================
--- gcc-fsf-trunk-quilt.orig/gcc/config/mips/mips.md	2013-11-19 01:48:08.000000000 +0000
+++ gcc-fsf-trunk-quilt/gcc/config/mips/mips.md	2013-11-19 01:48:25.787735722 +0000
@@ -881,15 +881,6 @@ 
 (define_mode_attr sqrt_condition
   [(SF "!ISA_MIPS1") (DF "!ISA_MIPS1") (V2SF "TARGET_SB1")])
 
-;; This attribute gives the conditions under which RECIP.fmt and RSQRT.fmt
-;; instructions can be used.  The MIPS32 and MIPS64 ISAs say that RECIP.D
-;; and RSQRT.D are unpredictable when doubles are stored in pairs of FPRs,
-;; so for safety's sake, we apply this restriction to all targets.
-(define_mode_attr recip_condition
-  [(SF "ISA_HAS_FP4")
-   (DF "ISA_HAS_FP4 && TARGET_FLOAT64")
-   (V2SF "TARGET_SB1")])
-
 ;; This code iterator allows signed and unsigned widening multiplications
 ;; to use the same template.
 (define_code_iterator any_extend [sign_extend zero_extend])
@@ -2501,7 +2492,8 @@ 
   "<divide_condition>"
 {
   if (const_1_operand (operands[1], <MODE>mode))
-    if (!(<recip_condition> && flag_unsafe_math_optimizations))
+    if (!(ISA_HAS_FP_RECIP_RSQRT (<MODE>mode)
+	  && flag_unsafe_math_optimizations))
       operands[1] = force_reg (<MODE>mode, operands[1]);
 })
 
@@ -2539,7 +2531,7 @@ 
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(div:ANYF (match_operand:ANYF 1 "const_1_operand" "")
 		  (match_operand:ANYF 2 "register_operand" "f")))]
-  "<recip_condition> && flag_unsafe_math_optimizations"
+  "ISA_HAS_FP_RECIP_RSQRT (<MODE>mode) && flag_unsafe_math_optimizations"
 {
   if (TARGET_FIX_SB1)
     return "recip.<fmt>\t%0,%2\;mov.<fmt>\t%0,%0";
@@ -2674,7 +2666,7 @@ 
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(div:ANYF (match_operand:ANYF 1 "const_1_operand" "")
 		  (sqrt:ANYF (match_operand:ANYF 2 "register_operand" "f"))))]
-  "<recip_condition> && flag_unsafe_math_optimizations"
+  "ISA_HAS_FP_RECIP_RSQRT (<MODE>mode) && flag_unsafe_math_optimizations"
 {
   if (TARGET_FIX_SB1)
     return "rsqrt.<fmt>\t%0,%2\;mov.<fmt>\t%0,%0";
@@ -2692,7 +2684,7 @@ 
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(sqrt:ANYF (div:ANYF (match_operand:ANYF 1 "const_1_operand" "")
 			     (match_operand:ANYF 2 "register_operand" "f"))))]
-  "<recip_condition> && flag_unsafe_math_optimizations"
+  "ISA_HAS_FP_RECIP_RSQRT (<MODE>mode) && flag_unsafe_math_optimizations"
 {
   if (TARGET_FIX_SB1)
     return "rsqrt.<fmt>\t%0,%2\;mov.<fmt>\t%0,%0";