diff mbox

Basic support for MIPS r5900

Message ID 87li6b1aia.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford June 15, 2013, 6:42 p.m. UTC 
"Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>
>> > I can't approve the Makefile.in bits.  I've cc'ed Ian, who's the libgcc
>> > maintainer.  Ian: the problem is that "_muldi3.o" on 64-bit targets
>> > is actually an implementation of __multi3.  Jürgen wants to have a
>> > __muldi3 too, with the same implementation as on 32-bit targets.
>>
>> My assumption is that target maintainers can approve target-specific
>> changes to libgcc, including Makefile changes.
>>
>> That said, it seems that this particular patch ought to mostly be in
>> libgcc/config/mips/t-mips, using LIB2FUNCS_EXCLUDE and LIB2ADD.  It's
>> not clear to me that libgcc/Makefile.in needs any changes, and in case
>> it should not be necessary for it to have anything like MIPSTYPE.  That
>> kind of thing belongs in config/mips.
>
> The code is now completely moved into libgcc/config/mips/t-mips and
> libgcc/config/mips/lib2funcs.c (new file).
> The code should now be easier to understand.
> I used the code from libgcc/config/m32c as example (e.g. same file name
> lib2funcs.c). I copied the file header (LGPL) from a file which I
> believed to be new and correct.

Thanks, this looks very clean.  It's good that the new file compiles
to nothing for ISA_HAS_DMULT/ISA_HAS_DDIV targets.  In that case though,
I think it should be added to LIB2ADD_ST rather than LIB2ADD.
Objects from LIB2ADD are included in libgcc.so, which needs to have
a stable interface, whereas LIB2ADD_ST is purely for libgcc.a,
where this kind of variation is OK.

Putting them in one file means they'll either all be pulled in or none will,
but I doubt the size is going to matter in practice, right?  Besides,
that kind of thing could easily be tweaked later if it shows up as a problem.

Also, I see you changed the patch so that mul<mode>3 tests ISA_HAS_<D>MULT
in the C body rather than in the condition.  Was that in response to my
previous comment about define_expand conditions?  Your first version was
right because mul<mode>3 is a public pattern that's exposed to optabs
(insn-opinit.c tests HAVE_mul<mode>3).  The other two define_expands
you mentioned are private to the MIPS port though and we never use
HAVE_* for them.  We only use them from places where the insns are
already known to be valid.

The ISA_HAS_DMUL3 part was redundant, sorry for not noticing it last time.

Does it still work with those changes, as below?  If so, I'll check it in.

Thanks,
Richard


gcc/
2013-06-15  Jürgen Urban  <JuergenUrban@gmx.de>

	* config/mips/mips.h (ISA_HAS_MUL3): Include TARGET_R5900.
	(ISA_HAS_MULT, ISA_HAS_DMULT, ISA_HAS_DIV, ISA_HAS_DDIV): New macros.
	* config/mips/mips.md (mul<mode>3, mul<mode>3_internal)
	(mul<mode>3_r4000): Require ISA_HAS_<D>MULT.
	(mul<mode>3_mul3): Handle TARGET_R5900.
	(mulsidi3_64bit_dmul): Remove redundant TARGET_64BIT test.
	(<su>muldi3_highpart, <su>muldi3_highpart_internal, <u>mulditi3)
	(<u>mulditi3_internal, <u>mulditi3_r4000): Require ISA_HAS_DMULT
	instead of TARGET_64BIT.
	(divmod<mode>4, udivmod<mode>4, <u>divmod<GPR:mode>4_hilo_<HILO:mode>):
	Require ISA_HAS_<D>DIV.

libgcc/
2013-06-15  Jürgen Urban  <JuergenUrban@gmx.de>

	* config/mips/lib2funcs.c: New file.
	* config/mips/t-mips (LIB2ADD_ST): Add it.

Comments

Jürgen Urban June 16, 2013, 3 p.m. UTC | #1 
Hello Richard,

> > The code is now completely moved into libgcc/config/mips/t-mips and
> > libgcc/config/mips/lib2funcs.c (new file).
> > The code should now be easier to understand.
> > I used the code from libgcc/config/m32c as example (e.g. same file name
> > lib2funcs.c). I copied the file header (LGPL) from a file which I
> > believed to be new and correct.
>
> Thanks, this looks very clean.  It's good that the new file compiles
> to nothing for ISA_HAS_DMULT/ISA_HAS_DDIV targets.  In that case though,
> I think it should be added to LIB2ADD_ST rather than LIB2ADD.
> Objects from LIB2ADD are included in libgcc.so, which needs to have
> a stable interface, whereas LIB2ADD_ST is purely for libgcc.a,
> where this kind of variation is OK.

Regarding LIB2ADD vs. LIB2ADD_ST:
At the moment I can't run a test with shared libraries, but it seems to work when linking a program using shared libraries. There is no undefined reference to __muldi3 when linking shared and using the toolchain for r5900.

> Putting them in one file means they'll either all be pulled in or none will,
> but I doubt the size is going to matter in practice, right?  Besides,
> that kind of thing could easily be tweaked later if it shows up as a problem.
>
> Also, I see you changed the patch so that mul<mode>3 tests ISA_HAS_<D>MULT
> in the C body rather than in the condition.  Was that in response to my
> previous comment about define_expand conditions?

Yes.

> Your first version was
> right because mul<mode>3 is a public pattern that's exposed to optabs
> (insn-opinit.c tests HAVE_mul<mode>3).  The other two define_expands
> you mentioned are private to the MIPS port though and we never use
> HAVE_* for them.  We only use them from places where the insns are
> already known to be valid.

OK, now I know the missing details.

> The ISA_HAS_DMUL3 part was redundant, sorry for not noticing it last time.

Yes, I thought it must be changed, but you are right.

> Does it still work with those changes, as below?  If so, I'll check it in.

I tested it. It is still working. So the patch is OK, please check it in.

Best regards
Jürgen
Richard Sandiford June 16, 2013, 7:31 p.m. UTC | #2 
"Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> Does it still work with those changes, as below?  If so, I'll check it in.
>
> I tested it. It is still working. So the patch is OK, please check it in.

OK, I've applied this and the config.gcc patch.

Thanks,
Richard
diff mbox

Patch

Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	2013-06-15 14:55:16.985850737 +0100
+++ gcc/config/mips/mips.h	2013-06-15 19:32:51.637536044 +0100
@@ -807,6 +807,7 @@  #define ISA_HAS_BRANCHLIKELY	(!ISA_MIPS1
 #define ISA_HAS_MUL3		((TARGET_MIPS3900                       \
 				  || TARGET_MIPS5400			\
 				  || TARGET_MIPS5500			\
+				  || TARGET_MIPS5900			\
 				  || TARGET_MIPS7000			\
 				  || TARGET_MIPS9000			\
 				  || TARGET_MAD				\
@@ -821,6 +822,22 @@  #define ISA_HAS_DMUL3		(TARGET_64BIT
 				 && TARGET_OCTEON			\
 				 && !TARGET_MIPS16)
 
+/* ISA supports instructions DMULT and DMULTU. */
+#define ISA_HAS_DMULT		(TARGET_64BIT && !TARGET_MIPS5900)
+
+/* ISA supports instructions MULT and MULTU.
+   This is always true, but the macro is needed for ISA_HAS_<D>MULT
+   in mips.md.  */
+#define ISA_HAS_MULT		(1)
+
+/* ISA supports instructions DDIV and DDIVU. */
+#define ISA_HAS_DDIV		(TARGET_64BIT && !TARGET_MIPS5900)
+
+/* 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		(1)
+
 #define ISA_HAS_DIV3		((TARGET_LOONGSON_2EF			\
 				  || TARGET_LOONGSON_3A)		\
 				 && !TARGET_MIPS16)
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2013-06-15 14:55:16.985850737 +0100
+++ gcc/config/mips/mips.md	2013-06-15 19:13:40.678334979 +0100
@@ -1468,7 +1468,7 @@  (define_expand "mul<mode>3"
   [(set (match_operand:GPR 0 "register_operand")
 	(mult:GPR (match_operand:GPR 1 "register_operand")
 		  (match_operand:GPR 2 "register_operand")))]
-  ""
+  "ISA_HAS_<D>MULT"
 {
   rtx lo;
 
@@ -1514,7 +1514,7 @@  (define_insn "mul<mode>3_mul3"
 {
   if (which_alternative == 1)
     return "<d>mult\t%1,%2";
-  if (<MODE>mode == SImode && TARGET_MIPS3900)
+  if (<MODE>mode == SImode && (TARGET_MIPS3900 || TARGET_MIPS5900))
     return "mult\t%0,%1,%2";
   return "<d>mul\t%0,%1,%2";
 }
@@ -1548,7 +1548,7 @@  (define_insn "mul<mode>3_internal"
   [(set (match_operand:GPR 0 "muldiv_target_operand" "=l")
 	(mult:GPR (match_operand:GPR 1 "register_operand" "d")
 		  (match_operand:GPR 2 "register_operand" "d")))]
-  "!TARGET_FIX_R4000"
+  "ISA_HAS_<D>MULT && !TARGET_FIX_R4000"
   "<d>mult\t%1,%2"
   [(set_attr "type" "imul")
    (set_attr "mode" "<MODE>")])
@@ -1558,7 +1558,7 @@  (define_insn "mul<mode>3_r4000"
 	(mult:GPR (match_operand:GPR 1 "register_operand" "d")
 		  (match_operand:GPR 2 "register_operand" "d")))
    (clobber (match_scratch:GPR 3 "=l"))]
-  "TARGET_FIX_R4000"
+  "ISA_HAS_<D>MULT && TARGET_FIX_R4000"
   "<d>mult\t%1,%2\;mflo\t%0"
   [(set_attr "type" "imul")
    (set_attr "mode" "<MODE>")
@@ -2025,7 +2025,7 @@  (define_insn "mulsidi3_64bit_dmul"
 	(mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" "d"))
 		 (sign_extend:DI (match_operand:SI 2 "register_operand" "d"))))
    (clobber (match_scratch:DI 3 "=l"))]
-  "TARGET_64BIT && ISA_HAS_DMUL3"
+  "ISA_HAS_DMUL3"
   "dmul\t%0,%1,%2"
   [(set_attr "type" "imul3")
    (set_attr "mode" "DI")])
@@ -2179,7 +2179,7 @@  (define_expand "<su>muldi3_highpart"
 	  (mult:TI (any_extend:TI (match_operand:DI 1 "register_operand"))
 		   (any_extend:TI (match_operand:DI 2 "register_operand")))
 	  (const_int 64))))]
-  "TARGET_64BIT && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
+  "ISA_HAS_DMULT && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
 {
   if (TARGET_MIPS16)
     emit_insn (gen_<su>muldi3_highpart_split (operands[0], operands[1],
@@ -2198,7 +2198,7 @@  (define_insn_and_split "<su>muldi3_highp
 		   (any_extend:TI (match_operand:DI 2 "register_operand" "d")))
 	  (const_int 64))))
    (clobber (match_scratch:DI 3 "=l"))]
-  "TARGET_64BIT
+  "ISA_HAS_DMULT
    && !TARGET_MIPS16
    && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
   { return TARGET_FIX_R4000 ? "dmult<u>\t%1,%2\n\tmfhi\t%0" : "#"; }
@@ -2234,7 +2234,7 @@  (define_expand "<u>mulditi3"
   [(set (match_operand:TI 0 "register_operand")
 	(mult:TI (any_extend:TI (match_operand:DI 1 "register_operand"))
 		 (any_extend:TI (match_operand:DI 2 "register_operand"))))]
-  "TARGET_64BIT && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
+  "ISA_HAS_DMULT && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
 {
   rtx hilo;
 
@@ -2256,7 +2256,7 @@  (define_insn "<u>mulditi3_internal"
   [(set (match_operand:TI 0 "muldiv_target_operand" "=x")
 	(mult:TI (any_extend:TI (match_operand:DI 1 "register_operand" "d"))
 		 (any_extend:TI (match_operand:DI 2 "register_operand" "d"))))]
-  "TARGET_64BIT
+  "ISA_HAS_DMULT
    && !TARGET_FIX_R4000
    && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
   "dmult<u>\t%1,%2"
@@ -2268,7 +2268,7 @@  (define_insn "<u>mulditi3_r4000"
 	(mult:TI (any_extend:TI (match_operand:DI 1 "register_operand" "d"))
 		 (any_extend:TI (match_operand:DI 2 "register_operand" "d"))))
    (clobber (match_scratch:TI 3 "=x"))]
-  "TARGET_64BIT
+  "ISA_HAS_DMULT
    && TARGET_FIX_R4000
    && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
   "dmult<u>\t%1,%2\;mflo\t%L0\;mfhi\t%M0"
@@ -2564,7 +2564,7 @@  (define_insn_and_split "divmod<mode>4"
    (set (match_operand:GPR 3 "register_operand" "=d")
 	(mod:GPR (match_dup 1)
 		 (match_dup 2)))]
-  "!TARGET_FIX_VR4120"
+  "ISA_HAS_<D>DIV && !TARGET_FIX_VR4120"
   "#"
   "&& ((TARGET_MIPS16 && cse_not_expected) || reload_completed)"
   [(const_int 0)]
@@ -2587,7 +2587,7 @@  (define_insn_and_split "udivmod<mode>4"
    (set (match_operand:GPR 3 "register_operand" "=d")
 	(umod:GPR (match_dup 1)
 		  (match_dup 2)))]
-  ""
+  "ISA_HAS_<D>DIV"
   "#"
   "(TARGET_MIPS16 && cse_not_expected) || reload_completed"
   [(const_int 0)]
@@ -2633,7 +2633,7 @@  (define_insn "<u>divmod<GPR:mode>4_hilo_
 	  [(any_div:GPR (match_operand:GPR 1 "register_operand" "d")
 			(match_operand:GPR 2 "register_operand" "d"))]
 	  UNSPEC_SET_HILO))]
-  ""
+  "ISA_HAS_<GPR:D>DIV"
   { return mips_output_division ("<GPR:d>div<u>\t%.,%1,%2", operands); }
   [(set_attr "type" "idiv")
    (set_attr "mode" "<GPR:MODE>")])
Index: libgcc/config/mips/lib2funcs.c
===================================================================
--- /dev/null	2013-06-13 15:03:44.173086200 +0100
+++ libgcc/config/mips/lib2funcs.c	2013-06-15 14:56:08.965279026 +0100
@@ -0,0 +1,44 @@ 
+/* libgcc routines for MIPS
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   DMULT/DDIV replacement support by Juergen Urban, JuergenUrban@gmx.de.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+<http://www.gnu.org/licenses/>.  */
+
+#if defined(__mips64) && defined(_MIPS_ARCH_R5900)
+
+/* Build DI version of libgcc functions. */
+#define LIBGCC2_UNITS_PER_WORD 4
+
+/* The following function is needed when !ISA_HAS_DMULT. */
+#define L_muldi3
+
+/* The following functions are needed when !ISA_HAS_DDIV. */
+#define L_divdi3
+#define L_moddi3
+#define L_udivdi3
+#define L_umoddi3
+#define L_udivmoddi4
+
+/* Use generic definition of functions. */
+#include "libgcc2.c"
+
+#endif
Index: libgcc/config/mips/t-mips
===================================================================
--- libgcc/config/mips/t-mips	2013-06-15 14:55:16.985850737 +0100
+++ libgcc/config/mips/t-mips	2013-06-15 14:56:08.965279026 +0100
@@ -4,3 +4,5 @@  FPBIT = true
 FPBIT_CFLAGS = -DQUIET_NAN_NEGATED
 DPBIT = true
 DPBIT_CFLAGS = -DQUIET_NAN_NEGATED
+
+LIB2ADD_ST += $(srcdir)/config/mips/lib2funcs.c