diff mbox

MIPS: Fix generation of DIV.G and MOD.G for Loongson targets.

Message ID A614194ED15B4844BC4C9FB7F21FCD92225439B1@HHMAIL01.hh.imgtec.org
State New
Headers show

Commit Message

Toma Tabacu Jan. 17, 2017, 4:54 p.m. UTC
> Maciej Rozycki writes:
> >  This ought to be handled then, likely by adding Loongson-specific RTL
> > insns matching the `divmod<mode>4' and `udivmod<mode>4' expanders.  It
> > may be as simple as say (conceptually, untested):
> >
> > (define_insn "<u>divmod<GPR:mode>4_loongson"
> >   [(set (match_operand:GPR 0 "register_operand" "=d")
> > 	(any_div:GPR (match_operand:GPR 1 "register_operand" "d")
> > 		     (match_operand:GPR 2 "register_operand" "d")))
> >    (set (match_operand:GPR 3 "register_operand" "=d")
> > 	(any_mod:GPR (match_dup 1)
> > 		     (match_dup 2)))]
> >   "TARGET_LOONGSON_2EF"
> > {
> >   return mips_output_division
> >     ("<GPR:d>div<u>.g\t%0,%1,%2\;<GPR:d>mod<u>.g\t%3,%1,%2",
> operands);
> > }
> >   [(set_attr "type" "idiv")
> >    (set_attr "mode" "<GPR:MODE>")])
> >
> > although any final fix will have to take an instruction count adjustment
> > into account too, as `mips_idiv_insns' won't as it stands handle the new
> > case.

Thanks for the tip Maciej!
I will tackle that issue in a separate patch.

Matthew Fortune writes:
> 
> Sounds good. I'd prefer to get the testsuite clean first then improve the
> code quality as a later step since it is not a regression and we are
> a few days off stage 4.
> 
> In terms of the patch then the ISA_HAS_DIV3 macro is not currently used so
> I suggest that instead it is renamed to ISA_AVOID_DIV_HILO and then use
> that macro in the definition of ISA_HAS_DIV and ISA_HAS_DDIV to turn
> off the DIV/DDIV instructions.
> 
> The ISA_HAS_DIV3 should have been cleaned up when R6 was added as it is
> ambiguous and could refer to multiple variants of 3-reg operand DIV now
> rather than just Loongson's.
> 
> Thanks,
> Matthew

I believe the patch below fits the description.
I've also added a (too?) succinct explanation for the ISA_AVOID_DIV_HILO macro.

Tested with mips-mti-elf.

Regards,
Toma

gcc/ChangeLog:

	* config/mips/mips.h: Add macro to prevent generation of regular
	(D)DIV(U) instructions for Loongson.

Comments

Matthew Fortune Jan. 18, 2017, 9:52 a.m. UTC | #1
Toma Tabacu <Toma.Tabacu@imgtec.com> writes:
> Matthew Fortune writes:
> >
> > Sounds good. I'd prefer to get the testsuite clean first then improve the
> > code quality as a later step since it is not a regression and we are
> > a few days off stage 4.
> >
> > In terms of the patch then the ISA_HAS_DIV3 macro is not currently used so
> > I suggest that instead it is renamed to ISA_AVOID_DIV_HILO and then use
> > that macro in the definition of ISA_HAS_DIV and ISA_HAS_DDIV to turn
> > off the DIV/DDIV instructions.
> >
> > The ISA_HAS_DIV3 should have been cleaned up when R6 was added as it is
> > ambiguous and could refer to multiple variants of 3-reg operand DIV now
> > rather than just Loongson's.
> >
> > Thanks,
> > Matthew
> 
> I believe the patch below fits the description.
> I've also added a (too?) succinct explanation for the ISA_AVOID_DIV_HILO macro.
> 
> Tested with mips-mti-elf.
> 
> Regards,
> Toma
> 
> gcc/ChangeLog:
> 
> 	* config/mips/mips.h: Add macro to prevent generation of regular
> 	(D)DIV(U) instructions for Loongson.

The changelog needs to be more detailed about what changed and can be
less detailed about why:

 	* config/mips/mips.h (ISA_HAS_DIV3): Remove unused macro.
	(ISA_AVOID_DIV_HILO): New macro.
	(ISA_HAS_DIV): Use new ISA_AVOID_DIV_HILO macro.
	(ISA_HAS_DDIV): Likewise.

> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
> index f91b43d..e21e7d8 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -967,19 +967,24 @@ struct mips_cpu_info {
>  /* ISA supports instructions DMUL, DMULU, DMUH, DMUHU.  */
>  #define ISA_HAS_R6DMUL		(TARGET_64BIT && mips_isa_rev >= 6)
> 
> +/* For Loongson, it is preferable to use the Loongson-specific division and
> +   modulo instructions instead of the regular (D)DIV(U) instruction, because
> +   the former are faster and also have the effect of reducing code size.  */

Might want to put 'also can have the effect' given they don't yet.

> +#define ISA_AVOID_DIV_HILO	((TARGET_LOONGSON_2EF			\
> +				  || TARGET_LOONGSON_3A)		\
> +				 && !TARGET_MIPS16)
> +
>  /* ISA supports instructions DDIV and DDIVU. */
>  #define ISA_HAS_DDIV		(TARGET_64BIT				\
>  				 && !TARGET_MIPS5900			\
> +				 && !ISA_AVOID_DIV_HILO			\
>  				 && mips_isa_rev <= 5)
> 
>  /* ISA supports instructions DIV and DIVU.
>     This is always true, but the macro is needed for ISA_HAS_<D>DIV
>     in mips.md.  */
> -#define ISA_HAS_DIV		(mips_isa_rev <= 5)
> -
> -#define ISA_HAS_DIV3		((TARGET_LOONGSON_2EF			\
> -				  || TARGET_LOONGSON_3A)		\
> -				 && !TARGET_MIPS16)
> +#define ISA_HAS_DIV		(!ISA_AVOID_DIV_HILO			\
> +				 && mips_isa_rev <= 5)
> 
>  /* ISA supports instructions DIV, DIVU, MOD and MODU.  */
>  #define ISA_HAS_R6DIV		(mips_isa_rev >= 6)

Apart from those changes this looks OK to me.

Matthew
Toma Tabacu Jan. 18, 2017, 11:08 a.m. UTC | #2
> From: Matthew Fortune
> 
> Apart from those changes this looks OK to me.
> 
> Matthew

Thanks.
Committed as r244570.

Regards,
Toma
diff mbox

Patch

diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index f91b43d..e21e7d8 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -967,19 +967,24 @@  struct mips_cpu_info {
 /* ISA supports instructions DMUL, DMULU, DMUH, DMUHU.  */
 #define ISA_HAS_R6DMUL		(TARGET_64BIT && mips_isa_rev >= 6)
 
+/* For Loongson, it is preferable to use the Loongson-specific division and
+   modulo instructions instead of the regular (D)DIV(U) instruction, because
+   the former are faster and also have the effect of reducing code size.  */
+#define ISA_AVOID_DIV_HILO	((TARGET_LOONGSON_2EF			\
+				  || TARGET_LOONGSON_3A)		\
+				 && !TARGET_MIPS16)
+
 /* ISA supports instructions DDIV and DDIVU. */
 #define ISA_HAS_DDIV		(TARGET_64BIT				\
 				 && !TARGET_MIPS5900			\
+				 && !ISA_AVOID_DIV_HILO			\
 				 && mips_isa_rev <= 5)
 
 /* ISA supports instructions DIV and DIVU.
    This is always true, but the macro is needed for ISA_HAS_<D>DIV
    in mips.md.  */
-#define ISA_HAS_DIV		(mips_isa_rev <= 5)
-
-#define ISA_HAS_DIV3		((TARGET_LOONGSON_2EF			\
-				  || TARGET_LOONGSON_3A)		\
-				 && !TARGET_MIPS16)
+#define ISA_HAS_DIV		(!ISA_AVOID_DIV_HILO			\
+				 && mips_isa_rev <= 5)
 
 /* ISA supports instructions DIV, DIVU, MOD and MODU.  */
 #define ISA_HAS_R6DIV		(mips_isa_rev >= 6)