diff mbox

Aw: Re: [PATCH] Basic support for MIPS r5900

Message ID trinity-d20eff1d-14b2-4351-82e3-0e186c9ca625-1370993071149@3capp-gmx-bs35
State New
Headers show

Commit Message

Jürgen Urban June 11, 2013, 11:24 p.m. UTC 
Hello Richard,

> > How much other changes will be currently accepted here? There is other
> > stuff which I want to prepare and submit here, e.g.:
> > 1. disable use of dmult and ddiv (ABI n32).
> > 2. use trunc.w.s instead of cvt.w.s (to get single float working for
> > normal range calculations; i.e. calculating without inf or nan).
> > 3. fix use of ll/sc in libgomp, either increase mips ISA level or use
> > syscall (which is broken in Linux 2.6.35.4).
> > 4. fix libgcc to build a real muldi3 function for ABI n32 (not the
> > multi3 function which is stored in muldi3.o file).
> > 5. add support for configure parameters --float=single and
> > --float=double in addition to --float=soft and --float=hard.
> > 6. rework floating point to support single float with ABI n32 (either
> > break the ABI or store floating point values in general purpose
> > registers like soft float).
> > 7. change libgcc or mips.md in way so that the non IEEE 754 compatible
> > FPU of the r5900 gets compatible.
>
> Well, I'm afraid that's hard to say in advance.  It really depends
> on what the changes look like.  (1) and (2) sound harmless enough,
> although (1) should probably only be done in conjunction with (4).
> I'm not sure what (3) involves.  (5) sounds like a good idea.
> (6) is worth doing, but anything ABI-related gets extra-paranoid
> treatment. :-)

The attached patch fixes (1) and (4). This makes mips64r5900el usable with r5900. If (4) is a problem (i.e. patching libgcc/Makefile.in), it would be good if at least (1) is accepted.
The patch for mips.md after line 1992 (adds TARGET_64BIT) is a more general fix. This is not needed for r5900 support, but I think this should be fixed.
The same applies for patch after 2233 (adds ISA_HAS_DMULT). The fix here would be also adding TARGET_64BIT, but for r5900 we need ISA_HAS_DMULT here. Other changes in mips.md should not change the behaviour of GCC on non r5900 toolchains.

Best regards
Jürgen

Comments

Richard Sandiford June 12, 2013, 7:16 p.m. UTC | #1 
"Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> > How much other changes will be currently accepted here? There is other
>> > stuff which I want to prepare and submit here, e.g.:
>> > 1. disable use of dmult and ddiv (ABI n32).
>> > 2. use trunc.w.s instead of cvt.w.s (to get single float working for
>> > normal range calculations; i.e. calculating without inf or nan).
>> > 3. fix use of ll/sc in libgomp, either increase mips ISA level or use
>> > syscall (which is broken in Linux 2.6.35.4).
>> > 4. fix libgcc to build a real muldi3 function for ABI n32 (not the
>> > multi3 function which is stored in muldi3.o file).
>> > 5. add support for configure parameters --float=single and
>> > --float=double in addition to --float=soft and --float=hard.
>> > 6. rework floating point to support single float with ABI n32 (either
>> > break the ABI or store floating point values in general purpose
>> > registers like soft float).
>> > 7. change libgcc or mips.md in way so that the non IEEE 754 compatible
>> > FPU of the r5900 gets compatible.
>>
>> Well, I'm afraid that's hard to say in advance.  It really depends
>> on what the changes look like.  (1) and (2) sound harmless enough,
>> although (1) should probably only be done in conjunction with (4).
>> I'm not sure what (3) involves.  (5) sounds like a good idea.
>> (6) is worth doing, but anything ABI-related gets extra-paranoid
>> treatment. :-)
>
> The attached patch fixes (1) and (4). This makes mips64r5900el usable
> with r5900. If (4) is a problem (i.e. patching libgcc/Makefile.in), it
> would be good if at least (1) is accepted.

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.

I think (1) and (4) should go in together though.  (1) doesn't make much
sense without a libgcc function to back it up.

> The patch for mips.md after line 1992 (adds TARGET_64BIT) is a more
> general fix. This is not needed for r5900 support, but I think this
> should be fixed.
> The same applies for patch after 2233 (adds ISA_HAS_DMULT). The fix here
> would be also adding TARGET_64BIT, but for r5900 we need ISA_HAS_DMULT
> here.

The current state is actually deliberate.  define_expand conditions are
only ever used in HAVE_* macros, so whatever we put there will not get
tested.  I think it's less confusing to have no test than an unused one,
just like we try not to have constraints in define_expands.

The other bits of the config/mips patch look good, thanks.  A couple of
formatting niggles:

> +/* ISA supports instructions dmult and dmultu. */
> +#define ISA_HAS_DMULT           (TARGET_64BIT				\
> +				 && !TARGET_MIPS5900)
> +
> +/* ISA supports instructions mult and multu.
> +   This always supported, 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)

Please keep ISA_HAS_DMULT and ISA_HAS_DDIV on one line while they fit.
I prefer caps for insn names in the comments, but the code isn't yet
as consistent as it should be, sorry...

> +/* ISA supports instructions div and divu.
> +   This always supported, but the macro is needed for ISA_HAS_<D>DIV
> +   in mips.md.  */
> +#define ISA_HAS_DIV		(1)
> +
> +

Excess blank line here.

Thanks,
Richard
Ian Lance Taylor June 14, 2013, 4:24 a.m. UTC | #2 
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.

Ian
diff mbox

Patch

Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(Revision 199708)
+++ gcc/config/mips/mips.h	(Arbeitskopie)
@@ -807,6 +807,7 @@ 
 #define ISA_HAS_MUL3		((TARGET_MIPS3900                       \
 				  || TARGET_MIPS5400			\
 				  || TARGET_MIPS5500			\
+				  || TARGET_MIPS5900			\
 				  || TARGET_MIPS7000			\
 				  || TARGET_MIPS9000			\
 				  || TARGET_MAD				\
@@ -819,8 +820,28 @@ 
 /* ISA has a three-operand multiplication instruction.  */
 #define ISA_HAS_DMUL3		(TARGET_64BIT				\
 				 && TARGET_OCTEON			\
+				 && !TARGET_MIPS5900                    \
 				 && !TARGET_MIPS16)
 
+/* ISA supports instructions dmult and dmultu. */
+#define ISA_HAS_DMULT           (TARGET_64BIT				\
+				 && !TARGET_MIPS5900)
+
+/* ISA supports instructions mult and multu.
+   This always supported, 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 always supported, 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	(Revision 199708)
+++ gcc/config/mips/mips.md	(Arbeitskopie)
@@ -1481,7 +1481,7 @@ 
   [(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;
 
@@ -1527,7 +1527,7 @@ 
 {
   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";
 }
@@ -1561,7 +1561,7 @@ 
   [(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>")])
@@ -1571,7 +1571,7 @@ 
 	(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>")
@@ -1992,7 +1992,7 @@ 
 	(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
 		 (any_extend:DI (match_operand:SI 2 "register_operand"))))
    (clobber (match_operand:DI 3 "register_operand"))]
-  ""
+  "TARGET_64BIT"
 {
   rtx hilo;
 
@@ -2038,7 +2038,7 @@ 
 	(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")])
@@ -2192,7 +2192,7 @@ 
 	  (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],
@@ -2211,7 +2211,7 @@ 
 		   (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" : "#"; }
@@ -2233,7 +2233,7 @@ 
 	  (mult:TI (any_extend:TI (match_operand:DI 1 "register_operand"))
 		   (any_extend:TI (match_operand:DI 2 "register_operand")))
 	  (const_int 64))))]
-  ""
+  "ISA_HAS_DMULT"
 {
   rtx hilo;
 
@@ -2247,7 +2247,7 @@ 
   [(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;
 
@@ -2269,7 +2269,7 @@ 
   [(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"
@@ -2281,7 +2281,7 @@ 
 	(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"
@@ -2577,7 +2577,7 @@ 
    (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)]
@@ -2599,7 +2599,7 @@ 
    (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)]
@@ -2617,7 +2617,7 @@ 
   [(set (match_operand:GPR 0 "register_operand")
 	(any_mod:GPR (match_operand:GPR 1 "register_operand")
 		     (match_operand:GPR 2 "register_operand")))]
-  ""
+  "ISA_HAS_<D>DIV"
 {
   rtx hilo;
 
@@ -2644,7 +2644,7 @@ 
 	  [(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/Makefile.in
===================================================================
--- libgcc/Makefile.in	(Revision 199708)
+++ libgcc/Makefile.in	(Arbeitskopie)
@@ -293,6 +293,11 @@ 
 inst_libdir = $(libsubdir)$(MULTISUBDIR)
 inst_slibdir = $(slibdir)$(MULTIOSSUBDIR)
 
+# Get mips type: __mips or __mips64 is defined as GCC macro:
+MIPSTYPE := $(shell $(CC) $(CFLAGS) -dM -E - < /dev/null | \
+		grep -e "\<__mips\>" -e "\<__mips64\>" | \
+		(read define type value; echo $$type))
+
 gcc_compile_bare = $(CC) $(INTERNAL_CFLAGS)
 compile_deps = -MT $@ -MD -MP -MF $(basename $@).dep
 gcc_compile = $(gcc_compile_bare) -o $@ $(compile_deps)
@@ -401,8 +406,9 @@ 
 LIB2ADDEHSHARED += $(srcdir)/emutls.c
 
 # Library members defined in libgcc2.c.
-lib2funcs = _muldi3 _negdi2 _lshrdi3 _ashldi3 _ashrdi3 _cmpdi2 _ucmpdi2	   \
-	    _clear_cache _trampoline __main _absvsi2 \
+lib2difuncs = _muldi3
+lib2funcs = $(lib2difuncs) _negdi2 _lshrdi3 _ashldi3 _ashrdi3 _cmpdi2	   \
+	    _ucmpdi2 _clear_cache _trampoline __main _absvsi2		   \
 	    _absvdi2 _addvsi3 _addvdi3 _subvsi3 _subvdi3 _mulvsi3 _mulvdi3 \
 	    _negvsi2 _negvdi2 _ctors _ffssi2 _ffsdi2 _clz _clzsi2 _clzdi2  \
 	    _ctzsi2 _ctzdi2 _popcount_tab _popcountsi2 _popcountdi2	   \
@@ -427,7 +433,8 @@ 
 
 # These might cause a divide overflow trap and so are compiled with
 # unwinder info.
-LIB2_DIVMOD_FUNCS = _divdi3 _moddi3 _udivdi3 _umoddi3 _udiv_w_sdiv _udivmoddi4
+LIB2_DIVMODDI_FUNCS = _divdi3 _moddi3 _udivdi3 _umoddi3 _udivmoddi4
+LIB2_DIVMOD_FUNCS = $(LIB2_DIVMODDI_FUNCS) _udiv_w_sdiv
 
 # Remove any objects from lib2funcs and LIB2_DIVMOD_FUNCS that are
 # defined as optimized assembly code in LIB1ASMFUNCS or as C code
@@ -459,13 +466,29 @@ 
 $(lib2funcs-o): %$(objext): $(srcdir)/libgcc2.c
 	$(gcc_compile) -DL$* -c $< $(vis_hide)
 libgcc-objects += $(lib2funcs-o)
+ifeq ($(MIPSTYPE),__mips64)
+# Build functions needed by MIPS r5900.
+lib2difuncs-o = $(patsubst %,%$(objext),$(addsuffix _32bit,$(lib2difuncs)))
+$(lib2difuncs-o): %$(objext): $(srcdir)/libgcc2.c
+	$(gcc_compile) -DL$(subst _32bit,,$*) -DLIBGCC2_UNITS_PER_WORD=4 \
+	  -c $< $(vis_hide)
+libgcc-objects += $(lib2difuncs-o)
+endif
 
 ifeq ($(enable_shared),yes)
 lib2funcs-s-o = $(patsubst %,%_s$(objext),$(lib2funcs))
 $(lib2funcs-s-o): %_s$(objext): $(srcdir)/libgcc2.c
 	$(gcc_s_compile) -DL$* -c $<
 libgcc-s-objects += $(lib2funcs-s-o)
+ifeq ($(MIPSTYPE),__mips64)
+# Build functions needed by MIPS r5900.
+lib2difuncs-s-o = $(patsubst %,%_s$(objext),$(addsuffix _32bit,$(lib2difuncs)))
+$(lib2difuncs-s-o): %_s$(objext): $(srcdir)/libgcc2.c
+	$(gcc_s_compile) -DL$(subst _32bit,,$*) -DLIBGCC2_UNITS_PER_WORD=4 \
+	  -c $<
+libgcc-s-objects += $(lib2difuncs-s-o)
 endif
+endif
 
 ifneq ($(LIB2_SIDITI_CONV_FUNCS),)
 # Build libgcc2.c for each conversion function, with a specific
@@ -501,6 +524,15 @@ 
 	$(gcc_compile) -DL$* -c $< \
 	  $(LIB2_DIVMOD_EXCEPTION_FLAGS) $(vis_hide)
 libgcc-objects += $(lib2-divmod-o)
+ifeq ($(MIPSTYPE),__mips64)
+# Build functions needed by MIPS r5900.
+lib2-divmoddi-o = \
+	$(patsubst %,%$(objext),$(addsuffix _32bit,$(LIB2_DIVMODDI_FUNCS)))
+$(lib2-divmoddi-o): %$(objext): $(srcdir)/libgcc2.c
+	$(gcc_compile) -DL$(subst _32bit,,$*) -DLIBGCC2_UNITS_PER_WORD=4 \
+	  -c $< $(LIB2_DIVMOD_EXCEPTION_FLAGS) $(vis_hide)
+libgcc-objects += $(lib2-divmoddi-o)
+endif
 
 ifeq ($(enable_shared),yes)
 lib2-divmod-s-o = $(patsubst %,%_s$(objext),$(LIB2_DIVMOD_FUNCS))
@@ -508,7 +540,16 @@ 
 	$(gcc_s_compile) -DL$* -c $< \
 	  $(LIB2_DIVMOD_EXCEPTION_FLAGS)
 libgcc-s-objects += $(lib2-divmod-s-o)
+ifeq ($(MIPSTYPE),__mips64)
+# Build functions needed by MIPS r5900.
+lib2-divmoddi-s-o = \
+	$(patsubst %,%_s$(objext),$(addsuffix _32bit,$(LIB2_DIVMODDI_FUNCS)))
+$(lib2-divmoddi-s-o): %_s$(objext): $(srcdir)/libgcc2.c
+	$(gcc_s_compile) -DL$(subst _32bit,,$*) -DLIBGCC2_UNITS_PER_WORD=4 \
+	  -c $< $(LIB2_DIVMOD_EXCEPTION_FLAGS)
+libgcc-s-objects += $(lib2-divmoddi-s-o)
 endif
+endif
 
 ifeq ($(TPBIT),)
 # _sf_to_tf and _df_to_tf require tp-bit.c being compiled in.