public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch, mips] Patch to control the use of integer madd/msub instructions
@ 2013-03-22 23:16 Steve Ellcey 
  2013-03-23 14:50 ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: Steve Ellcey  @ 2013-03-22 23:16 UTC (permalink / raw)
  To: gcc-patches

While testing GCC on a 74k MIPS chip I noticed that by default the -mtune=74k*
flags cause GCC to not use the integer madd/msub instructions.  According to
the checkin comments these were found to cause a performance hit over using
individual mult and add/sub instructions.  I think there are some programs
though where using madd/msub would be a win on the 74k and I would like to
have a flag to allow users to override the default behaviour (either turning
it on for 74k or turning it off for other achitectures).  This patch allows
-mimadd or -mno-imadd to override the default behaviour but does not change
that default behaviour.

OK for checkin?

Steve Ellcey
sellcey@imgtec.com


2013-03-22  Steve Ellcey  <sellcey@mips.com>

	* config/mips/mips.md (mimadd): New flag for integer madd/msub.
	* config/mips/mips.h (GENERATE_MADD_MSUB): Check -mimadd flag.


diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 0acce14..62a7701 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -875,7 +875,9 @@ struct mips_cpu_info {
 				 && !TARGET_MIPS16)
 
 /* Integer multiply-accumulate instructions should be generated.  */
-#define GENERATE_MADD_MSUB      (ISA_HAS_MADD_MSUB && !TUNE_74K)
+#define GENERATE_MADD_MSUB	(ISA_HAS_MADD_MSUB \
+				 && (target_flags_explicit & MASK_IMADD \
+					? TARGET_IMADD : !TUNE_74K))
 
 /* ISA has floating-point madd and msub instructions 'd = a * b [+-] c'.  */
 #define ISA_HAS_FP_MADD4_MSUB4  ISA_HAS_FP4
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index d8ef2e7..6b3024b 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -58,6 +58,10 @@ mmad
 Target Report Var(TARGET_MAD)
 Use PMC-style 'mad' instructions
 
+mimadd
+Target Report Mask(IMADD)
+Use integer madd/msub instructions
+
 march=
 Target RejectNegative Joined Var(mips_arch_option) ToLower Enum(mips_arch_opt_value)
 -march=ISA	Generate code for the given ISA

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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-22 23:16 [patch, mips] Patch to control the use of integer madd/msub instructions Steve Ellcey 
@ 2013-03-23 14:50 ` Richard Sandiford
  2013-03-25 16:12   ` Steve Ellcey
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2013-03-23 14:50 UTC (permalink / raw)
  To: Steve Ellcey ; +Cc: gcc-patches

"Steve Ellcey " <sellcey@imgtec.com> writes:
> While testing GCC on a 74k MIPS chip I noticed that by default the -mtune=74k*
> flags cause GCC to not use the integer madd/msub instructions.  According to
> the checkin comments these were found to cause a performance hit over using
> individual mult and add/sub instructions.  I think there are some programs
> though where using madd/msub would be a win on the 74k and I would like to
> have a flag to allow users to override the default behaviour (either turning
> it on for 74k or turning it off for other achitectures).  This patch allows
> -mimadd or -mno-imadd to override the default behaviour but does not change
> that default behaviour.

This is similar in spirit to -mbranch-likely.  It'd be good for consistency
if they were defined in a similar style.  I think that means removing
!TARGET_MIPS16 from ISA_HAS_MADD_MSUB and instead having:

#define GENERATE_MADD_MSUB      (TARGET_IMADD && !TARGET_MIPS16)

There would also be:

#define PTF_AVOID_IMADD 0x2

which should be included in the 74k description, and a block similar to
the MASK_BRANCHLIKELY one in mips_option_override.  There needs to be
documentation in invoke.texi.

But -- sorry for the soapbox speech -- it would be better to retune
so that new options aren't needed.  I'm assuming you're testing against
the same microarchitecture that the original 74k authors were.  If so,
it seems like -mimadd is just an option for choosing between two bad
implementations.  One uses MADD and MSUB unconditionally (contrary to
the experience of the original authors) and the other never uses it
at all (contrary to your experience).

That's not enough reason to reject the patch, just saying :-)

Thanks,
Richard

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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-23 14:50 ` Richard Sandiford
@ 2013-03-25 16:12   ` Steve Ellcey
  2013-03-25 16:46     ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: Steve Ellcey @ 2013-03-25 16:12 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

On Sat, 2013-03-23 at 14:50 +0000, Richard Sandiford wrote:

> This is similar in spirit to -mbranch-likely.  It'd be good for consistency
> if they were defined in a similar style.  I think that means removing
> !TARGET_MIPS16 from ISA_HAS_MADD_MSUB and instead having:
> 
> #define GENERATE_MADD_MSUB      (TARGET_IMADD && !TARGET_MIPS16)
> 
> There would also be:
> 
> #define PTF_AVOID_IMADD 0x2
> 
> which should be included in the 74k description, and a block similar to
> the MASK_BRANCHLIKELY one in mips_option_override.  There needs to be
> documentation in invoke.texi.

I can do it this way if you want, I was using -mllsc as my template for
how to implement this.  Do you think the -mllsc flag should be
implemented in the same way as -mbranch-likely?

> But -- sorry for the soapbox speech -- it would be better to retune
> so that new options aren't needed.  I'm assuming you're testing against
> the same microarchitecture that the original 74k authors were.  If so,
> it seems like -mimadd is just an option for choosing between two bad
> implementations.  One uses MADD and MSUB unconditionally (contrary to
> the experience of the original authors) and the other never uses it
> at all (contrary to your experience).
> 
> That's not enough reason to reject the patch, just saying :-)

I agree that the 74k should only be using the integer madd/msub
instruction where it makes sense but I think having a flag to allow the
user to override it is still a good thing because the compiler won't
always be right.  Actually, one of my reasons for adding this flag is to
make it easier for me to do 74k runs with and without madd/msub and see
where we are using (but shouldn't) and hopefully improve the current
implementation.

Steve Ellcey
sellcey@mips.com



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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-25 16:12   ` Steve Ellcey
@ 2013-03-25 16:46     ` Richard Sandiford
  2013-03-25 20:53       ` Steve Ellcey
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2013-03-25 16:46 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches

Steve Ellcey <sellcey@imgtec.com> writes:
> On Sat, 2013-03-23 at 14:50 +0000, Richard Sandiford wrote:
>> This is similar in spirit to -mbranch-likely.  It'd be good for consistency
>> if they were defined in a similar style.  I think that means removing
>> !TARGET_MIPS16 from ISA_HAS_MADD_MSUB and instead having:
>> 
>> #define GENERATE_MADD_MSUB      (TARGET_IMADD && !TARGET_MIPS16)
>> 
>> There would also be:
>> 
>> #define PTF_AVOID_IMADD 0x2
>> 
>> which should be included in the 74k description, and a block similar to
>> the MASK_BRANCHLIKELY one in mips_option_override.  There needs to be
>> documentation in invoke.texi.
>
> I can do it this way if you want, I was using -mllsc as my template for
> how to implement this.  Do you think the -mllsc flag should be
> implemented in the same way as -mbranch-likely?

-mllsc is a little different in that it can be used even when the
ISA doesn't support it (thanks to kernel emulation).  -mimadd isn't
like that though: we only want to use MADD/MSUB if the ISA has it.
So I think it makes sense to leave -mllsc as it is but do -mimadd
in the same way as -mbranch-likely.

Thanks,
Richard

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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-25 16:46     ` Richard Sandiford
@ 2013-03-25 20:53       ` Steve Ellcey
  2013-03-25 21:50         ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: Steve Ellcey @ 2013-03-25 20:53 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

On Mon, 2013-03-25 at 16:45 +0000, Richard Sandiford wrote:

> -mllsc is a little different in that it can be used even when the
> ISA doesn't support it (thanks to kernel emulation).  -mimadd isn't
> like that though: we only want to use MADD/MSUB if the ISA has it.
> So I think it makes sense to leave -mllsc as it is but do -mimadd
> in the same way as -mbranch-likely.
> 
> Thanks,
> Richard

OK, Here is a patch that implements -mimadd in the same manner as
-mbranch-likely.

Steve Ellcey
sellcey@imgtec.com


2013-03-25  Steve Ellcey  <sellcey@mips.com>

	* config/mips/mmips-cpus.def (74kc, 74kf2_1, 74kf, 74kf, 74kf1_1,
	74kfx, 74kx, 74kf3_2): Add PTF_AVOID_IMADD.
	* config/mips/mips.c (mips_option_override): Set IMADD default.
	* config/mips/mips.h (PTF_AVOID_IMADD): New.
	(ISA_HAS_MADD_MSUB): Remove MIPS16 check.
	(GENERATE_MADD_MSUB): Remove TUNE_74K check, add MIPS16 check.
	* config/mips/mips.md (mimadd): New flag for integer madd/msub.

diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index 93c305a..c920c73 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -119,13 +119,13 @@ MIPS_CPU ("34kfx", PROCESSOR_24KF1_1, 33, 0)
 MIPS_CPU ("34kx", PROCESSOR_24KF1_1, 33, 0)
 MIPS_CPU ("34kn", PROCESSOR_24KC, 33, 0)  /* 34K with MT but no DSP.  */
 
-MIPS_CPU ("74kc", PROCESSOR_74KC, 33, 0) /* 74K with DSPr2.  */
-MIPS_CPU ("74kf2_1", PROCESSOR_74KF2_1, 33, 0)
-MIPS_CPU ("74kf", PROCESSOR_74KF2_1, 33, 0)
-MIPS_CPU ("74kf1_1", PROCESSOR_74KF1_1, 33, 0)
-MIPS_CPU ("74kfx", PROCESSOR_74KF1_1, 33, 0)
-MIPS_CPU ("74kx", PROCESSOR_74KF1_1, 33, 0)
-MIPS_CPU ("74kf3_2", PROCESSOR_74KF3_2, 33, 0)
+MIPS_CPU ("74kc", PROCESSOR_74KC, 33, PTF_AVOID_IMADD) /* 74K with DSPr2.  */
+MIPS_CPU ("74kf2_1", PROCESSOR_74KF2_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kf", PROCESSOR_74KF2_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kf1_1", PROCESSOR_74KF1_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kfx", PROCESSOR_74KF1_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kx", PROCESSOR_74KF1_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kf3_2", PROCESSOR_74KF3_2, 33, PTF_AVOID_IMADD)
 
 MIPS_CPU ("1004kc", PROCESSOR_24KC, 33, 0) /* 1004K with MT/DSP.  */
 MIPS_CPU ("1004kf2_1", PROCESSOR_24KF2_1, 33, 0)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 252e828..0aaf4c6 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16607,6 +16607,21 @@ mips_option_override (void)
     warning (0, "the %qs architecture does not support branch-likely"
 	     " instructions", mips_arch_info->name);
 
+  /* If the user hasn't specified -mimadd or -mno-imadd set
+     MASK_IMADD based on the target architecture and tuning
+     flags.  */
+  if ((target_flags_explicit & MASK_IMADD) == 0)
+    {
+      if (ISA_HAS_MADD_MSUB &&
+          (mips_tune_info->tune_flags & PTF_AVOID_IMADD) == 0)
+	target_flags |= MASK_IMADD;
+      else
+	target_flags &= ~MASK_IMADD;
+    }
+  else if (TARGET_IMADD && !ISA_HAS_MADD_MSUB)
+    warning (0, "the %qs architecture does not support madd or msub"
+	     " instructions", mips_arch_info->name);
+
   /* The effect of -mabicalls isn't defined for the EABI.  */
   if (mips_abi == ABI_EABI && TARGET_ABICALLS)
     {
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 0acce14..534ea26 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -42,13 +42,17 @@ extern int target_flags_explicit;
 #define ABI_EABI 3
 #define ABI_O64  4
 
-/* Masks that affect tuning.
-
-   PTF_AVOID_BRANCHLIKELY
-	Set if it is usually not profitable to use branch-likely instructions
-	for this target, typically because the branches are always predicted
-	taken and so incur a large overhead when not taken.  */
-#define PTF_AVOID_BRANCHLIKELY 0x1
+/* Masks that affect tuning.  */
+
+/* Set PTF_AVOID_BRANCHLIKELY if is usually not profitable to use
+   branch-likely instructions for this target, typically because
+   the branches are always predicted taken and so incur a large
+   overhead when not taken.  */
+#define PTF_AVOID_BRANCHLIKELY	0x1
+/* Set PTF_AVOID_IMADD if it is usually not profitable to use the
+   integer madd or msub instructions because of the overhead of
+   getting the result out of the HI/LO registers.  */
+#define PTF_AVOID_IMADD		0x2
 
 /* Information about one recognized processor.  Defined here for the
    benefit of TARGET_CPU_CPP_BUILTINS.  */
@@ -868,14 +872,13 @@ struct mips_cpu_info {
 				 && !TARGET_MIPS16)
 
 /* ISA has integer multiply-accumulate instructions, madd and msub.  */
-#define ISA_HAS_MADD_MSUB	((ISA_MIPS32				\
-				  || ISA_MIPS32R2			\
-				  || ISA_MIPS64				\
-				  || ISA_MIPS64R2)			\
-				 && !TARGET_MIPS16)
+#define ISA_HAS_MADD_MSUB	(ISA_MIPS32				\
+				 || ISA_MIPS32R2			\
+				 || ISA_MIPS64				\
+				 || ISA_MIPS64R2)
 
 /* Integer multiply-accumulate instructions should be generated.  */
-#define GENERATE_MADD_MSUB      (ISA_HAS_MADD_MSUB && !TUNE_74K)
+#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
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index d8ef2e7..6b3024b 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -58,6 +58,10 @@ mmad
 Target Report Var(TARGET_MAD)
 Use PMC-style 'mad' instructions
 
+mimadd
+Target Report Mask(IMADD)
+Use integer madd/msub instructions
+
 march=
 Target RejectNegative Joined Var(mips_arch_option) ToLower Enum(mips_arch_opt_value)
 -march=ISA	Generate code for the given ISA



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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-25 20:53       ` Steve Ellcey
@ 2013-03-25 21:50         ` Richard Sandiford
  2013-03-25 22:50           ` Steve Ellcey
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2013-03-25 21:50 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches

Steve Ellcey <sellcey@imgtec.com> writes:
> On Mon, 2013-03-25 at 16:45 +0000, Richard Sandiford wrote:
>
>> -mllsc is a little different in that it can be used even when the
>> ISA doesn't support it (thanks to kernel emulation).  -mimadd isn't
>> like that though: we only want to use MADD/MSUB if the ISA has it.
>> So I think it makes sense to leave -mllsc as it is but do -mimadd
>> in the same way as -mbranch-likely.
>> 
>> Thanks,
>> Richard
>
> OK, Here is a patch that implements -mimadd in the same manner as
> -mbranch-likely.

It still needs the invoke.texi documentation. :-) Looks good otherwise,
just a very small nit:

> -/* Masks that affect tuning.
> -
> -   PTF_AVOID_BRANCHLIKELY
> -	Set if it is usually not profitable to use branch-likely instructions
> -	for this target, typically because the branches are always predicted
> -	taken and so incur a large overhead when not taken.  */
> -#define PTF_AVOID_BRANCHLIKELY 0x1
> +/* Masks that affect tuning.  */
> +
> +/* Set PTF_AVOID_BRANCHLIKELY if is usually not profitable to use
> +   branch-likely instructions for this target, typically because
> +   the branches are always predicted taken and so incur a large
> +   overhead when not taken.  */
> +#define PTF_AVOID_BRANCHLIKELY	0x1
> +/* Set PTF_AVOID_IMADD if it is usually not profitable to use the
> +   integer madd or msub instructions because of the overhead of
> +   getting the result out of the HI/LO registers.  */
> +#define PTF_AVOID_IMADD		0x2

It wasn't obvious with just one PTK_*, but the idea was to lay this out
in the same way as the mips-protos.h enums.  I.e.:

/* Masks that affect tuning.

   PTF_AVOID_BRANCHLIKELY
	Set if it is usually not profitable to use branch-likely instructions
	for this target, typically because the branches are always predicted
	taken and so incur a large overhead when not taken.

   PTF_AVOID_IMADD
	Set if it is usually not profitable to use the integer MADD or MSUB
	instructions because of the overhead of getting the result out of
	the HI/LO registers.  */
#define PTF_AVOID_BRANCHLIKELY	0x1
#define PTF_AVOID_IMADD		0x2

That's trivial enough not to need a retest, but please post the
invoke.texi patch.

Thanks,
Richard

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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-25 21:50         ` Richard Sandiford
@ 2013-03-25 22:50           ` Steve Ellcey
  2013-03-25 22:59             ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: Steve Ellcey @ 2013-03-25 22:50 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

On Mon, 2013-03-25 at 21:50 +0000, Richard Sandiford wrote:

> That's trivial enough not to need a retest, but please post the
> invoke.texi patch.
> 
> Thanks,
> Richard

Ah, yes I always forget the documentation.  Here is the complete patch.
It has the invoke.texi change at the bottom and I fixed up the comments
in mips.h.

Steve Ellcey
sellcey@imgtec.com



2013-03-25  Steve Ellcey  <sellcey@mips.com>

	* config/mips/mmips-cpus.def (74kc, 74kf2_1, 74kf, 74kf, 74kf1_1,
	74kfx, 74kx, 74kf3_2): Add PTF_AVOID_IMADD.
	* config/mips/mips.c (mips_option_override): Set IMADD default.
	* config/mips/mips.h (PTF_AVOID_IMADD): New.
	(ISA_HAS_MADD_MSUB): Remove MIPS16 check.
	(GENERATE_MADD_MSUB): Remove TUNE_74K check, add MIPS16 check.
	* config/mips/mips.md (mimadd): New flag for integer madd/msub.
	* doc/invoke.texi (-mimadd/-mno-imadd): New.
diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index 1cc1999..9e5fd16 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -121,13 +121,13 @@ MIPS_CPU ("34kfx", PROCESSOR_24KF1_1, 33, 0)
 MIPS_CPU ("34kx", PROCESSOR_24KF1_1, 33, 0)
 MIPS_CPU ("34kn", PROCESSOR_24KC, 33, 0)  /* 34K with MT but no DSP.  */
 
-MIPS_CPU ("74kc", PROCESSOR_74KC, 33, 0) /* 74K with DSPr2.  */
-MIPS_CPU ("74kf2_1", PROCESSOR_74KF2_1, 33, 0)
-MIPS_CPU ("74kf", PROCESSOR_74KF2_1, 33, 0)
-MIPS_CPU ("74kf1_1", PROCESSOR_74KF1_1, 33, 0)
-MIPS_CPU ("74kfx", PROCESSOR_74KF1_1, 33, 0)
-MIPS_CPU ("74kx", PROCESSOR_74KF1_1, 33, 0)
-MIPS_CPU ("74kf3_2", PROCESSOR_74KF3_2, 33, 0)
+MIPS_CPU ("74kc", PROCESSOR_74KC, 33, PTF_AVOID_IMADD) /* 74K with DSPr2.  */
+MIPS_CPU ("74kf2_1", PROCESSOR_74KF2_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kf", PROCESSOR_74KF2_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kf1_1", PROCESSOR_74KF1_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kfx", PROCESSOR_74KF1_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kx", PROCESSOR_74KF1_1, 33, PTF_AVOID_IMADD)
+MIPS_CPU ("74kf3_2", PROCESSOR_74KF3_2, 33, PTF_AVOID_IMADD)
 
 MIPS_CPU ("1004kc", PROCESSOR_24KC, 33, 0) /* 1004K with MT/DSP.  */
 MIPS_CPU ("1004kf2_1", PROCESSOR_24KF2_1, 33, 0)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index e4ab271..e3469da 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16862,6 +16862,21 @@ mips_option_override (void)
     warning (0, "the %qs architecture does not support branch-likely"
 	     " instructions", mips_arch_info->name);
 
+  /* If the user hasn't specified -mimadd or -mno-imadd set
+     MASK_IMADD based on the target architecture and tuning
+     flags.  */
+  if ((target_flags_explicit & MASK_IMADD) == 0)
+    {
+      if (ISA_HAS_MADD_MSUB &&
+          (mips_tune_info->tune_flags & PTF_AVOID_IMADD) == 0)
+	target_flags |= MASK_IMADD;
+      else
+	target_flags &= ~MASK_IMADD;
+    }
+  else if (TARGET_IMADD && !ISA_HAS_MADD_MSUB)
+    warning (0, "the %qs architecture does not support madd or msub"
+	     " instructions", mips_arch_info->name);
+
   /* The effect of -mabicalls isn't defined for the EABI.  */
   if (mips_abi == ABI_EABI && TARGET_ABICALLS)
     {
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 0db3698..dd694f3 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -47,8 +47,15 @@ extern int target_flags_explicit;
    PTF_AVOID_BRANCHLIKELY
 	Set if it is usually not profitable to use branch-likely instructions
 	for this target, typically because the branches are always predicted
-	taken and so incur a large overhead when not taken.  */
-#define PTF_AVOID_BRANCHLIKELY 0x1
+	taken and so incur a large overhead when not taken.
+
+   PTF_AVOID_IMADD
+	Set if it is usually not profitable to use the integer MADD or MSUB
+	instructions because of the overhead of getting the result out of
+	the HI/LO registers.  */
+
+#define PTF_AVOID_BRANCHLIKELY	0x1
+#define PTF_AVOID_IMADD		0x2
 
 /* Information about one recognized processor.  Defined here for the
    benefit of TARGET_CPU_CPP_BUILTINS.  */
@@ -874,14 +881,13 @@ struct mips_cpu_info {
 				 && !TARGET_MIPS16)
 
 /* ISA has integer multiply-accumulate instructions, madd and msub.  */
-#define ISA_HAS_MADD_MSUB	((ISA_MIPS32				\
-				  || ISA_MIPS32R2			\
-				  || ISA_MIPS64				\
-				  || ISA_MIPS64R2)			\
-				 && !TARGET_MIPS16)
+#define ISA_HAS_MADD_MSUB	(ISA_MIPS32				\
+				 || ISA_MIPS32R2			\
+				 || ISA_MIPS64				\
+				 || ISA_MIPS64R2)
 
 /* Integer multiply-accumulate instructions should be generated.  */
-#define GENERATE_MADD_MSUB      (ISA_HAS_MADD_MSUB && !TUNE_74K)
+#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
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index f9e88b3..e11710d 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -58,6 +58,10 @@ mmad
 Target Report Var(TARGET_MAD)
 Use PMC-style 'mad' instructions
 
+mimadd
+Target Report Mask(IMADD)
+Use integer madd/msub instructions
+
 march=
 Target RejectNegative Joined Var(mips_arch_option) ToLower Enum(mips_arch_opt_value)
 -march=ISA	Generate code for the given ISA
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9b8b36a..3054e5c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -766,7 +766,7 @@ Objective-C and Objective-C++ Dialects}.
 -mcheck-zero-division  -mno-check-zero-division @gol
 -mdivide-traps  -mdivide-breaks @gol
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
--mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
+-mmad -mno-mad -mimadd -mno-imadd -mfused-madd  -mno-fused-madd  -nocpp @gol
 -mfix-24k -mno-fix-24k @gol
 -mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
 -mfix-r10000 -mno-fix-r10000  -mfix-vr4120  -mno-fix-vr4120 @gol
@@ -16481,6 +16481,15 @@ This option has no effect on abicalls code.  The default is
 Enable (disable) use of the @code{mad}, @code{madu} and @code{mul}
 instructions, as provided by the R4650 ISA@.
 
+@item -mimadd
+@itemx -mno-imadd
+@opindex mimadd
+@opindex mno-imadd
+Enable (disable) use of the @code{madd} and @code{msub} integer
+instructions.  The default is @option{-mimadd} on architectures
+that support @code{madd} and @code{msub} except for the 74k 
+architecture where it was found to generate slower code.
+
 @item -mfused-madd
 @itemx -mno-fused-madd
 @opindex mfused-madd




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

* Re: [patch, mips] Patch to control the use of integer madd/msub instructions
  2013-03-25 22:50           ` Steve Ellcey
@ 2013-03-25 22:59             ` Richard Sandiford
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2013-03-25 22:59 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches

Steve Ellcey <sellcey@imgtec.com> writes:
> 	* config/mips/mmips-cpus.def (74kc, 74kf2_1, 74kf, 74kf, 74kf1_1,
> 	74kfx, 74kx, 74kf3_2): Add PTF_AVOID_IMADD.
> 	* config/mips/mips.c (mips_option_override): Set IMADD default.
> 	* config/mips/mips.h (PTF_AVOID_IMADD): New.
> 	(ISA_HAS_MADD_MSUB): Remove MIPS16 check.
> 	(GENERATE_MADD_MSUB): Remove TUNE_74K check, add MIPS16 check.
> 	* config/mips/mips.md (mimadd): New flag for integer madd/msub.
> 	* doc/invoke.texi (-mimadd/-mno-imadd): New.

OK, thanks.

Richard

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

end of thread, other threads:[~2013-03-25 22:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-22 23:16 [patch, mips] Patch to control the use of integer madd/msub instructions Steve Ellcey 
2013-03-23 14:50 ` Richard Sandiford
2013-03-25 16:12   ` Steve Ellcey
2013-03-25 16:46     ` Richard Sandiford
2013-03-25 20:53       ` Steve Ellcey
2013-03-25 21:50         ` Richard Sandiford
2013-03-25 22:50           ` Steve Ellcey
2013-03-25 22:59             ` Richard Sandiford

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).