Re: [PATCH] Basic support for MIPS r5900
Jürgen Urban writes: > Am 25.06.2013 um 23:24 schrieb Richard Sandiford : >> Jürgen Urban writes: >>> Am 23.06.2013 um 22:21 schrieb Richard Sandiford Do you want to stick with 128-bit long doubles for PS2, or would you prefer to do what SDE does? >>> >>> I prefer shorter long doubles because of the performance impact. For 32 >>> bit hard float we may need it. Can you change it or do I need to submit >>> a patch? >> >> OK, could you give this a go? > > I tested it and it is working as expected. Now applied, thanks. Richard
Re: [PATCH] Basic support for MIPS r5900
Hello Richard, Am 25.06.2013 um 23:24 schrieb Richard Sandiford : > Jürgen Urban writes: >> Am 23.06.2013 um 22:21 schrieb Richard Sandiford >> : In the native PS2SDK (i.e. no Linux) I detected that there are undefined references to `__fixtfsi', `__floatsitf', `__subtf3', `__multf3', __extenddftf2', `__lttf2' and `__gttf2'. These are the functions for handling "long double". There is a fix needed for this in libgcc. I attached 2 patches to this e-mail. There should be only one of the patches used to fix this. The first one fixes the problem for all mips systems. The second one fixes this only for r5900. I think it is a general problem. I assume the undefined references could appear on all mips64 systems which are not Linux. So the first patch should be applied. >>> >>> The second patch is right. Not all targets have 128-bit long doubles, >>> so t-tpbit is intentionally an opt-in rather than opt-out thing. >>> >>> E.g. mips*-sde-elf and mips*-mti-elf both use: >>> >>> #undef LONG_DOUBLE_TYPE_SIZE >>> #define LONG_DOUBLE_TYPE_SIZE 64 >>> >>> to get a form of n32 in which long doubles are the same as doubles. >>> >>> Do you want to stick with 128-bit long doubles for PS2, or would you >>> prefer to do what SDE does? >> >> I prefer shorter long doubles because of the performance impact. For 32 >> bit hard float we may need it. Can you change it or do I need to submit >> a patch? > > OK, could you give this a go? I tested it and it is working as expected. Best regards Jürgen
Re: [PATCH] Basic support for MIPS r5900
Jürgen Urban writes:
> Am 23.06.2013 um 22:21 schrieb Richard Sandiford :
>>> In the native PS2SDK (i.e. no Linux) I detected that there are undefined
>>> references to `__fixtfsi', `__floatsitf', `__subtf3', `__multf3',
>>> __extenddftf2', `__lttf2' and `__gttf2'. These are the functions for
>>> handling "long double".
>>> There is a fix needed for this in libgcc. I attached 2 patches to this
>>> e-mail. There should be only one of the patches used to fix this.
>>> The first one fixes the problem for all mips systems. The second one
>>> fixes this only for r5900. I think it is a general problem. I assume the
>>> undefined references could appear on all mips64 systems which are not
>>> Linux. So the first patch should be applied.
>>
>> The second patch is right. Not all targets have 128-bit long doubles,
>> so t-tpbit is intentionally an opt-in rather than opt-out thing.
>>
>> E.g. mips*-sde-elf and mips*-mti-elf both use:
>>
>> #undef LONG_DOUBLE_TYPE_SIZE
>> #define LONG_DOUBLE_TYPE_SIZE 64
>>
>> to get a form of n32 in which long doubles are the same as doubles.
>>
>> Do you want to stick with 128-bit long doubles for PS2, or would you
>> prefer to do what SDE does?
>
> I prefer shorter long doubles because of the performance impact. For 32
> bit hard float we may need it. Can you change it or do I need to submit
> a patch?
OK, could you give this a go?
Thanks,
Richard
gcc/
* config.gcc (mips*-mti-elf*, mips*-sde-elf*, mips64r5900-*-elf*)
(mips64r5900el-*-elf*): Include mips/n32-elf.h.
* config/mips/sde.h (LOCAL_LABEL_PREFIX, NO_DOLLAR_IN_LABEL)
(LONG_DOUBLE_TYPE_SIZE, LIBGCC2_LONG_DOUBLE_TYPE_SIZE): Move to...
* config/mips/n32-elf.h: ...this new file.
Index: gcc/config.gcc
===
--- gcc/config.gcc 2013-06-25 22:15:25.414731542 +0100
+++ gcc/config.gcc 2013-06-25 22:15:29.076761150 +0100
@@ -1850,12 +1850,12 @@ mips*-*-linux*) # Linux MIPS,
either
esac
;;
mips*-mti-elf*)
- tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/sde.h
mips/mti-elf.h"
+ tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h
mips/sde.h mips/mti-elf.h"
tmake_file="mips/t-mti-elf"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=33 MIPS_ABI_DEFAULT=ABI_32"
;;
mips*-sde-elf*)
- tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/sde.h"
+ tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h
mips/sde.h"
tmake_file="mips/t-sde"
extra_options="${extra_options} mips/sde.opt"
case "${with_newlib}" in
@@ -1937,7 +1937,7 @@ mips-*-elf* | mipsel-*-elf* | mipsr5900-
tmake_file="mips/t-elf"
;;
mips64r5900-*-elf* | mips64r5900el-*-elf*)
- tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h"
+ tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h"
tmake_file="mips/t-elf"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=3 MIPS_ABI_DEFAULT=ABI_N32"
;;
Index: gcc/config/mips/sde.h
===
--- gcc/config/mips/sde.h 2013-06-25 22:15:27.563748916 +0100
+++ gcc/config/mips/sde.h 2013-06-25 22:15:33.443796467 +0100
@@ -89,23 +89,6 @@ #define SIZE_TYPE "long unsigned int"
#undef PTRDIFF_TYPE
#define PTRDIFF_TYPE "long int"
-/* Use standard ELF-style local labels (not '$' as on early Irix). */
-#undef LOCAL_LABEL_PREFIX
-#define LOCAL_LABEL_PREFIX "."
-
-/* Use periods rather than dollar signs in special g++ assembler names. */
-#define NO_DOLLAR_IN_LABEL
-
-/* Currently we don't support 128bit long doubles, so for now we force
- n32 to be 64bit. */
-#undef LONG_DOUBLE_TYPE_SIZE
-#define LONG_DOUBLE_TYPE_SIZE 64
-
-#ifdef IN_LIBGCC2
-#undef LIBGCC2_LONG_DOUBLE_TYPE_SIZE
-#define LIBGCC2_LONG_DOUBLE_TYPE_SIZE 64
-#endif
-
/* Force all .init and .fini entries to be 32-bit, not mips16, so that
in a mixed environment they are all the same mode. The crti.asm and
crtn.asm files will also be compiled as 32-bit due to the
Index: gcc/config/mips/n32-elf.h
===
--- /dev/null 2013-06-17 18:11:16.259026486 +0100
+++ gcc/config/mips/n32-elf.h 2013-06-25 22:15:29.076761150 +0100
@@ -0,0 +1,35 @@
+/* Definitions of target machine for GNU compiler.
+ n32 for embedded systems.
+ Copyright (C) 2003-2013 Free Software Foundation, Inc.
+
+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
Re: [PATCH] Basic support for MIPS r5900
Hello Richard, Am 23.06.2013 um 22:21 schrieb Richard Sandiford : > "Jürgen Urban" writes: >> >> In the native PS2SDK (i.e. no Linux) I detected that there are undefined >> references to `__fixtfsi', `__floatsitf', `__subtf3', `__multf3', >> __extenddftf2', `__lttf2' and `__gttf2'. These are the functions for >> handling "long double". >> There is a fix needed for this in libgcc. I attached 2 patches to this >> e-mail. There should be only one of the patches used to fix this. >> The first one fixes the problem for all mips systems. The second one >> fixes this only for r5900. I think it is a general problem. I assume the >> undefined references could appear on all mips64 systems which are not >> Linux. So the first patch should be applied. > > The second patch is right. Not all targets have 128-bit long doubles, > so t-tpbit is intentionally an opt-in rather than opt-out thing. > > E.g. mips*-sde-elf and mips*-mti-elf both use: > > #undef LONG_DOUBLE_TYPE_SIZE > #define LONG_DOUBLE_TYPE_SIZE 64 > > to get a form of n32 in which long doubles are the same as doubles. > > Do you want to stick with 128-bit long doubles for PS2, or would you > prefer to do what SDE does? I prefer shorter long doubles because of the performance impact. For 32 bit hard float we may need it. Can you change it or do I need to submit a patch? Best regards Jürgen
Re: [PATCH] Basic support for MIPS r5900
"Jürgen Urban" writes: > Hello Richard, > >> >> 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. > > In the native PS2SDK (i.e. no Linux) I detected that there are undefined > references to `__fixtfsi', `__floatsitf', `__subtf3', `__multf3', > __extenddftf2', `__lttf2' and `__gttf2'. These are the functions for > handling "long double". > There is a fix needed for this in libgcc. I attached 2 patches to this > e-mail. There should be only one of the patches used to fix this. > The first one fixes the problem for all mips systems. The second one > fixes this only for r5900. I think it is a general problem. I assume the > undefined references could appear on all mips64 systems which are not > Linux. So the first patch should be applied. The second patch is right. Not all targets have 128-bit long doubles, so t-tpbit is intentionally an opt-in rather than opt-out thing. E.g. mips*-sde-elf and mips*-mti-elf both use: #undef LONG_DOUBLE_TYPE_SIZE #define LONG_DOUBLE_TYPE_SIZE 64 to get a form of n32 in which long doubles are the same as doubles. Do you want to stick with 128-bit long doubles for PS2, or would you prefer to do what SDE does? Thanks, Richard
Re: [PATCH] Basic support for MIPS r5900
Hello Richard,
> >> 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.
In the native PS2SDK (i.e. no Linux) I detected that there are undefined
references to `__fixtfsi', `__floatsitf', `__subtf3', `__multf3',
`__extenddftf2', `__lttf2' and `__gttf2'. These are the functions for handling
"long double".
There is a fix needed for this in libgcc. I attached 2 patches to this e-mail.
There should be only one of the patches used to fix this.
The first one fixes the problem for all mips systems. The second one fixes this
only for r5900. I think it is a general problem. I assume the undefined
references could appear on all mips64 systems which are not Linux. So the first
patch should be applied.
Checking for "ac_cv_sizeof_long_double" should work on all mips systems, but I
don't know why the check was only in the mips*-linux part of the file
libgcc/config.host.
Best regards
JürgenIndex: libgcc/config.host
===
--- libgcc/config.host (Revision 200155)
+++ libgcc/config.host (Arbeitskopie)
@@ -136,6 +136,9 @@
mips*-*-*)
cpu_type=mips
tmake_file=mips/t-mips
+ if test "${ac_cv_sizeof_long_double}" = 16; then
+ tmake_file="${tmake_file} mips/t-tpbit"
+ fi
;;
powerpc*-*-*)
cpu_type=rs6000
@@ -751,9 +754,6 @@
;;
esac
md_unwind_header=mips/linux-unwind.h
- if test "${ac_cv_sizeof_long_double}" = 16; then
- tmake_file="${tmake_file} mips/t-tpbit"
- fi
;;
mips*-sde-elf*)
tmake_file="$tmake_file mips/t-crtstuff mips/t-mips16"
Index: libgcc/config.host
===
--- libgcc/config.host (Revision 200155)
+++ libgcc/config.host (Arbeitskopie)
@@ -798,6 +798,9 @@
mips64r5900-*-elf* | mips64r5900el-*-elf*)
tmake_file="$tmake_file mips/t-elf mips/t-crtstuff"
extra_parts="$extra_parts crti.o crtn.o"
+ if test "${ac_cv_sizeof_long_double}" = 16; then
+ tmake_file="${tmake_file} mips/t-tpbit"
+ fi
;;
mips64vr-*-elf* | mips64vrel-*-elf*)
tmake_file="$tmake_file mips/t-elf mips/t-vr mips/t-crtstuff"
Re: Aw: Re: [PATCH] Basic support for MIPS r5900
"Jürgen Urban" 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
Aw: Re: [PATCH] Basic support for MIPS r5900
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 mul3 tests ISA_HAS_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 mul3 is a public pattern that's exposed to optabs > (insn-opinit.c tests HAVE_mul3). 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
Re: [PATCH] Basic support for MIPS r5900
"Jürgen Urban" writes: >> Richard Sandiford 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 mul3 tests ISA_HAS_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 mul3 is a public pattern that's exposed to optabs (insn-opinit.c tests HAVE_mul3). 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 * 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 (mul3, mul3_internal) (mul3_r4000): Require ISA_HAS_MULT. (mul3_mul3): Handle TARGET_R5900. (mulsidi3_64bit_dmul): Remove redundant TARGET_64BIT test. (muldi3_highpart, muldi3_highpart_internal, mulditi3) (mulditi3_internal, mulditi3_r4000): Require ISA_HAS_DMULT instead of TARGET_64BIT. (divmod4, udivmod4, divmod4_hilo_): Require ISA_HAS_DIV. libgcc/ 2013-06-15 Jürgen Urban * config/mips/lib2funcs.c: New file. * config/mips/t-mips (LIB2ADD_ST): Add it. 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_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_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 ==
Re: [PATCH] Basic support for MIPS r5900
On Fri, Jun 14, 2013 at 11:48 AM, "Jürgen Urban" wrote: > Hello Richard, > > I updated the patch as requested. > >> Richard Sandiford 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. > You can change it to whatever needed. I think this patch requires only target maintainer approval, not libgcc maintainer approval. Ian
Re: [PATCH] Basic support for MIPS r5900
Hello Richard,
I updated the patch as requested.
> Richard Sandiford 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.
You can change it to whatever needed.
Best regards
JürgenIndex: 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,25 @@
/* 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 is always supported, but the macro is needed for ISA_HAS_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 supported, but the macro is needed for ISA_HAS_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)
@@ -1498,9 +1498,11 @@
}
else if (TARGET_FIX_R4000)
emit_insn (gen_mul3_r4000 (operands[0], operands[1], operands[2]));
- else
+ else if (ISA_HAS_MULT)
emit_insn
(gen_mul3_internal (operands[0], operands[1], operands[2]));
+ else
+FAIL;
DONE;
})
@@ -1527,7 +1529,7 @@
{
if (which_alternative == 1)
return "mult\t%1,%2";
- if (mode == SImode && TARGET_MIPS3900)
+ if (mode == SImode && (TARGET_MIPS3900 || TARGET_MIPS5900))
return "mult\t%0,%1,%2";
return "mul\t%0,%1,%2";
}
@@ -1561,7 +1563,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_MULT && !TARGET_FIX_R4000"
"mult\t%1,%2"
[(set_attr "type" "imul")
(set_attr "mode" "")])
@@ -1571,7 +1573,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_MULT && TARGET_FIX_R4000"
"mult\t%1,%2\;mflo\t%0"
[(set_attr "type" "imul")
(set_attr "mode" "")
@@ -2038,7 +2040,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 +2194,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 && !( == ZERO_EXTEND && TARGET_FIX_VR4120)"
+ "ISA_HAS_DMULT && !( == ZERO_EXTEND && TARGET_FIX_VR4120)"
{
if (TARGET_MIPS16)
emit_insn (gen_muldi3_highpart_split (operands[0], operands[1],
@@ -2211,7 +2213,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
&& !( == ZERO_EXTEND && TARGET_FIX_VR4120)"
{ return TARGET_FIX_R4000 ? "dmult\t%1,%2\n\tmfhi\t%0" : "#"; }
@@ -2247,7 +2249,7 @@
[(
Re: Aw: Re: [PATCH] Basic support for MIPS r5900
Richard Sandiford 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
Re: Aw: Re: [PATCH] Basic support for MIPS r5900
"Jürgen Urban" 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_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_DIV > + in mips.md. */ > +#define ISA_HAS_DIV (1) > + > + Excess blank line here. Thanks, Richard
Aw: Re: [PATCH] Basic support for MIPS r5900
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ürgenIndex: 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_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_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_MULT"
{
rtx lo;
@@ -1527,7 +1527,7 @@
{
if (which_alternative == 1)
return "mult\t%1,%2";
- if (mode == SImode && TARGET_MIPS3900)
+ if (mode == SImode && (TARGET_MIPS3900 || TARGET_MIPS5900))
return "mult\t%0,%1,%2";
return "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_MULT && !TARGET_FIX_R4000"
"mult\t%1,%2"
[(set_attr "type" "imul")
(set_attr "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_MULT && TARGET_FIX_R4000"
"mult\t%1,%2\;mflo\t%0"
[(set_attr "type" "imul")
(set_attr "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
Re: [PATCH] Basic support for MIPS r5900
"Jürgen Urban" writes: > Hello Richard, > >> Thanks, looks good. The comments I have are only minor and seemed easier >> to spell out as a revised patch, attached below. The changes are: >> >> * removing the config.sub bit, which looked redundant. We already have >> the up-to-date upstream config.sub. >> >> * removing the target_cpu_default setting. I realise this was taken >> from mips64-elf, but it was redundant here, there, and elsewhere. >> I've just committed a patch to remove the existing cases. >> >> * removing the TUNE_5900 definition. I prefer not to define these >> kinds of macro until they're used. >> >> * removing the ISA_HAS_LDC1_SDC1 setting. I realise what you did >> describes the reality of the processor, but the problem is that >> the patch doesn't provide an alternative for 64-bit loads and >> stores when -mfp64 is used. That combination also isn't rejected, >> so we're likely to get an internal compiler error instead. >> >> This change shouldn't affect the soft-float case you describe. >> It also shouldn't be important for the single-float code. >> >> * tweaking the mips_reorg_process_insns comment slightly >> >> * fixing a few minor formatting issues >> >> Does this version look OK to you? I'll commit it if so. > > This is OK and the generated code is still working on the PS2 in my test run. Thanks, now applied. >> FWIW, the Cygnus/Red Hat version of the port just stuck with the R5900 >> behaviour and made GCC understand it (MODE_HAS_* & various other bits). >> This code was then updated and extended for the SPU. I'd have expected >> the support to be in reasonably good shape because of the SPU. > > I assume that you mean the cell processor of the PS3 and not the Sound > Processing Unit of the PS2. :-) > The macros MODE_HAS_* in the GCC look promising. You've probably already seen it, but there's also spu_single_format. Richard
Re: [PATCH] Basic support for MIPS r5900
Hello Richard, > Thanks, looks good. The comments I have are only minor and seemed easier > to spell out as a revised patch, attached below. The changes are: > > * removing the config.sub bit, which looked redundant. We already have > the up-to-date upstream config.sub. > > * removing the target_cpu_default setting. I realise this was taken > from mips64-elf, but it was redundant here, there, and elsewhere. > I've just committed a patch to remove the existing cases. > > * removing the TUNE_5900 definition. I prefer not to define these > kinds of macro until they're used. > > * removing the ISA_HAS_LDC1_SDC1 setting. I realise what you did > describes the reality of the processor, but the problem is that > the patch doesn't provide an alternative for 64-bit loads and > stores when -mfp64 is used. That combination also isn't rejected, > so we're likely to get an internal compiler error instead. > > This change shouldn't affect the soft-float case you describe. > It also shouldn't be important for the single-float code. > > * tweaking the mips_reorg_process_insns comment slightly > > * fixing a few minor formatting issues > > Does this version look OK to you? I'll commit it if so. This is OK and the generated code is still working on the PS2 in my test run. > > 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. :-) > > I'm not sure about (7) though. I'd imagine trying to get true IEEE > conformance out of the R5900 FPU would be pretty difficult and carry > quite a bit of runtime overhead, especially with the subnormal handling. > Is it really worth it? Do you have any particular use cases in mind? My target was to get existing C programs working (e.g. all mipsel programs from Debian 5 or 6, all mips64el programs from Fedora 12 or 13). The programs should work like programs on normal mipsel or mips64el Linux. I don't know what in the different programs is used. > FWIW, the Cygnus/Red Hat version of the port just stuck with the R5900 > behaviour and made GCC understand it (MODE_HAS_* & various other bits). > This code was then updated and extended for the SPU. I'd have expected > the support to be in reasonably good shape because of the SPU. I assume that you mean the cell processor of the PS3 and not the Sound Processing Unit of the PS2. The macros MODE_HAS_* in the GCC look promising. Best regards Jürgen
Re: [PATCH] Basic support for MIPS r5900
"Jürgen Urban" writes:
> after some months I reworked the patch for r5900. It would be nice if
> this could be accepted. The patch contains only changes to get basic
> support for MIPS r5900. It can be used to compile a working Linux kernel
> for the Playstation 2. It is also possible to get Linux programs working
> with software floating point and ABI o32. Other stuff like hardware
> floating point and ABI n32 is not fully supported yet.
Thanks, looks good. The comments I have are only minor and seemed easier
to spell out as a revised patch, attached below. The changes are:
* removing the config.sub bit, which looked redundant. We already have
the up-to-date upstream config.sub.
* removing the target_cpu_default setting. I realise this was taken
from mips64-elf, but it was redundant here, there, and elsewhere.
I've just committed a patch to remove the existing cases.
* removing the TUNE_5900 definition. I prefer not to define these
kinds of macro until they're used.
* removing the ISA_HAS_LDC1_SDC1 setting. I realise what you did
describes the reality of the processor, but the problem is that
the patch doesn't provide an alternative for 64-bit loads and
stores when -mfp64 is used. That combination also isn't rejected,
so we're likely to get an internal compiler error instead.
This change shouldn't affect the soft-float case you describe.
It also shouldn't be important for the single-float code.
* tweaking the mips_reorg_process_insns comment slightly
* fixing a few minor formatting issues
Does this version look OK to you? I'll commit it if so.
> 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. :-)
I'm not sure about (7) though. I'd imagine trying to get true IEEE
conformance out of the R5900 FPU would be pretty difficult and carry
quite a bit of runtime overhead, especially with the subnormal handling.
Is it really worth it? Do you have any particular use cases in mind?
FWIW, the Cygnus/Red Hat version of the port just stuck with the R5900
behaviour and made GCC understand it (MODE_HAS_* & various other bits).
This code was then updated and extended for the SPU. I'd have expected
the support to be in reasonably good shape because of the SPU.
Thanks,
Richard
gcc/
2013-06-03 Jürgen Urban
* config.gcc (mipsr5900-*-elf*, mipsr5900el-*-elf*, mips64r5900-*-elf*)
(mips64r5900el-*-elf*): New configurations.
* config/mips/mips-cpus.def (r5900): New processor.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.c (mips_rtx_cost_data): Add an R5900 entry.
(mips_issue_rate): Handle PROCESSOR_R5900.
(mips_reorg_process_insns): Force reorder mode for the R5900.
* config/mips/mips.h (TARGET_MIPS5900): Define.
(ISA_HAS_CONDMOVE, ISA_HAS_PREFETCH, ISA_HAS_HILO_INTERLOCKS): Include
TARGET_MIPS5900.
(ISA_HAS_LOAD_DELAY, ISA_HAS_XFER_DELAY, ISA_HAS_FCMP_DELAY): Exclude
TARGET_MIPS5900.
* config/mips/mips.md (processor): Add r5900.
(MOVECC): Disallow CCmode conditions for TARGET_MIPS5900.
libgcc/
2013-06-03 Jürgen Urban
* config.host (mipsr5900-*-elf*, mipsr5900el-*-elf*, mips64r5900-*-elf*)
(mips64r5900el-*-elf*): New configurations.
Index: gcc/config.gcc
===
--- gcc/config.gcc 2013-06-03 20:28:18.959933736 +0100
+++ gcc/config.gcc 2013-06-03 20:30:19.110139050 +0100
@@ -1935,10 +1935,15 @@ mipsisa64sb1-*-elf* | mipsisa64sb1el-*-e
tmake_file="mips/t-elf mips/t-sb1"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=64
MIPS_CPU_STRING_DEFAULT=\\\"sb1\\\" MIPS_ABI_DEFAULT=ABI_O64"
;;
-mips-*-elf* | mipsel-*-elf*)
+mips-*-elf* |
Re: [PATCH] Basic support for MIPS r5900
On 06/02/2013 01:45 PM, "Jürgen Urban" wrote: Hello, after some months I reworked the patch for r5900. It would be nice if this could be accepted. The patch contains only changes to get basic support for MIPS r5900. It can be used to compile a working Linux kernel for the Playstation 2. It is also possible to get Linux programs working with software floating point and ABI o32. Other stuff like hardware floating point and ABI n32 is not fully supported yet. 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. I'd like those more regularly working on the MIPS backend to chime in on final approval. However, it looks reasonable to me. jeff
