"Jürgen Urban" <juergenur...@gmx.de> writes: > ll, sc, dmult, ddiv, cvt.w.s, 64 bit FPU instructions. > ll and sc is disabled with "-mno-llsc" and works. > cvt.w.s is replaced by trunc.w.s. This seems to work.
Probably showing my ignorance, but I couldn't see this in the patch. > I disabled 64 bit FPU instructions by "-msoft-float". This works, but > using "-msingle-float" fails. This would be the better > configuration. There are still 64 bit FPU instructions used (e.g. "dmfc1 > $2,$f0" when using "long double" multiplication). So "-msingle-float" > doesn't seem to work on generic mips64-linux-gnu. Right. That combination hasn't really been defined. What happens for plain doubles? Do you pass those in FPRs or GPRs? > I tried to disable dmult and ddiv (see mips.md). Disabling worked, but > now muldi3 calls itself in libgcc2. I thought this should work, because > I got this working with GCC 4.3, but the latest GCC version is a > problem. multi3 is calling muldi3, so that muldi3 should be able to use > mulsi3, because it is the same C code in libgcc2. Can someone get me > some hints or comments? How can this be debugged? Not sure, sorry. > Does someone know how to enable TImode in MIPS ABI o32 (this doesn't > need to use the 128 bit instructions at the moment)? There is some old > code for the Playstation 2 which needs this. I know that TImode is > supported in ABI n32, but some code uses also the 32 bit FPU and FPU > registers are not available with "-msoft-float" in inline assembler. The n32 TImode support you mention uses pairs of GPRs, whereas I imagine you'd eventually want to use a single 128-bit GPR. Is that right? TImode in the current n32 sense doesn't really make practical sense for 32-bit targets. We'd be dealing with quad registers in that case. I think it would only make sense with TImode registers. ISTR the TImode registers being a can of worms, especially with the optimisation to only store the lower 64 bits if the upper 64 weren't used. (Am I remembering that right?) When you submit the TImode register support, please make the support itself a separate patch from the ABI changes. I.e. one patch to add TImode registers, one to add TImode to o32, one to add single-GPR TImode to n32, etc. For the record, I think all those patches would be too invasive this late into the 4.8 cycle so would have to wait for 4.9. Also, any ABI changes should be conditional on a new flag rather than keyed off the architecture. That flag would then be the default for your new configuration. > What is the best way to change the alignment to 128 bit for all > structures and stack in any MIPS ABI? Much old code for the Playstation > 2 expects this. The stack is STACK_BOUNDARY (already 128 for n32). Do you mean the padding of all structure types, or just global structure variables? If the former, it sounds like ROUND_TYPE_ALIGN, but also sounds scary :-) If the latter, it's DATA_ALIGNMENT. > @@ -15801,6 +15816,11 @@ mips_reorg_process_insns (void) > if (TARGET_FIX_VR4120 || TARGET_FIX_24K) > cfun->machine->all_noreorder_p = false; > > + /* Code compiled for R5900 can't be all noreorder because > + we rely on the assembler to work around some errata. */ > + if (TARGET_MIPS5900) > + cfun->machine->all_noreorder_p = false; > + > /* The same is true for -mfix-vr4130 if we might generate MFLO or > MFHI instructions. Note that we avoid using MFLO and MFHI if > the VR4130 MACC and DMACC instructions are available instead; Please fold this into the clause above it. > +/* Target supports instructions dmult and dmultu (integer). */ > +#define TARGET_HAS_DMULT (TARGET_64BIT \ > + && !TARGET_MIPS5900) Please use ISA_HAS_* for consistency with other macros. I think it'd be better to drop the '(integer)'. > +/* Target supports instructions mult and multu in 32 bit mode (integer). */ > +#define TARGET_HAS_MULT (mips_isa >= 1) ...and here drop 'in 32 bit mode (integer)'. 32-bit-mode isn't really relevant here. > +/* Target supports instructions dmult and dmultu (integer). */ > +#define TARGET_HAS_DDIV (TARGET_64BIT > \ > + && !TARGET_MIPS5900) Same as above. > +/* Target supports instructions mult and multu in 32 bit mode (integer). */ > +#define TARGET_HAS_DIV (mips_isa >= 1) Here too, plus "div" rather than "mult". > @@ -841,10 +859,10 @@ struct mips_cpu_info { > > /* ISA has the integer conditional move instructions introduced in mips4 and > ST Loongson 2E/2F. */ > -#define ISA_HAS_CONDMOVE (ISA_HAS_FP_CONDMOVE || TARGET_LOONGSON_2EF) > +#define ISA_HAS_CONDMOVE (ISA_HAS_FP_CONDMOVE || TARGET_LOONGSON_2EF > || TARGET_MIPS5900) GCC has a strict 80-column limit, so please make this: #define ISA_HAS_CONDMOVE (ISA_HAS_FP_CONDMOVE \ || TARGET_LOONGSON_2EF \ || TARGET_MIPS5900) > /* ISA has LDC1 and SDC1. */ > -#define ISA_HAS_LDC1_SDC1 (!ISA_MIPS1 && !TARGET_MIPS16) > +#define ISA_HAS_LDC1_SDC1 (!ISA_MIPS1 && !TARGET_MIPS16 && > !TARGET_MIPS5900) Same 3-line expansion here. > @@ -974,7 +993,11 @@ struct mips_cpu_info { > /* True if trunc.w.s and trunc.w.d are real (not synthetic) > instructions. Both require TARGET_HARD_FLOAT, and trunc.w.d > also requires TARGET_DOUBLE_FLOAT. */ > -#define ISA_HAS_TRUNC_W (!ISA_MIPS1) > +#define ISA_HAS_TRUNC_W_D (!ISA_MIPS1) > + > +/* True if trunc.w.s is real (not synthetic) instructions. > + Requires TARGET_HARD_FLOAT. */ > +#define ISA_HAS_TRUNC_W_S (ISA_HAS_TRUNC_W_D || TARGET_MIPS5900) First comment still describes both cases, so I think the second one is redundant. Just: /* True if trunc.w.s and trunc.w.d are real (not synthetic) instructions. Both require TARGET_HARD_FLOAT, and trunc.w.d also requires TARGET_DOUBLE_FLOAT. */ #define ISA_HAS_TRUNC_W_D (!ISA_MIPS1) #define ISA_HAS_TRUNC_W_S (ISA_HAS_TRUNC_W_D || TARGET_MIPS5900) > @@ -726,7 +727,7 @@ > ;; This mode iterator allows :MOVECC to be used anywhere that a > ;; conditional-move-type condition is needed. > (define_mode_iterator MOVECC [SI (DI "TARGET_64BIT") > - (CC "TARGET_HARD_FLOAT && > !TARGET_LOONGSON_2EF")]) > + (CC "TARGET_HARD_FLOAT && !TARGET_LOONGSON_2EF > && !TARGET_MIPS5900")]) Same three-line expansion here: (define_mode_iterator MOVECC [SI (DI "TARGET_64BIT") (CC "TARGET_HARD_FLOAT && !TARGET_LOONGSON_2EF && !TARGET_MIPS5900")]) > @@ -1900,7 +1901,7 @@ > [(set (match_operand:DI 0 "muldiv_target_operand" "=ka") > (mult:DI (any_extend:DI (match_operand:SI 1 "register_operand" "d")) > (any_extend:DI (match_operand:SI 2 "register_operand" "d"))))] > - "!TARGET_64BIT && (!TARGET_FIX_R4000 || ISA_HAS_DSP)" > + "(!TARGET_64BIT || (TARGET_64BIT && !TARGET_HAS_DMULT)) && > (!TARGET_FIX_R4000 || ISA_HAS_DSP)" > { > if (ISA_HAS_DSP_MULT) > return "mult<u>\t%q0,%1,%2"; Just: "!ISA_HAS_DMULTU && (!TARGET_FIX_R4000 || ISA_HAS_DSP)" Please update <u>mulsidi3_32bit_mips16 and <u>mulsidi3_32bit_r4000 in the same way. > @@ -1927,7 +1928,7 @@ > (any_extend:DI (match_operand:SI 2 "register_operand" "d")))) > (clobber (match_scratch:TI 3 "=x")) > (clobber (match_scratch:DI 4 "=d"))] > - "TARGET_64BIT && !TARGET_FIX_R4000 && !ISA_HAS_DMUL3 && !TARGET_MIPS16" > + "TARGET_64BIT && !TARGET_FIX_R4000 && !ISA_HAS_DMUL3 && !TARGET_MIPS16 && > TARGET_HAS_DMULT" > "#" > "&& reload_completed" > [(const_int 0)] Just: "ISA_HAS_DMULTU && !TARGET_FIX_R4000 && !ISA_HAS_DMUL3 && !TARGET_MIPS16" Please update <u>mulsidi3_64bit_mips16 in the same way. > @@ -2105,7 +2106,7 @@ > { > rtx hilo; > > - if (TARGET_64BIT) > + if (TARGET_64BIT && TARGET_HAS_DMULT) > { > hilo = gen_rtx_REG (TImode, MD_REG_FIRST); > emit_insn (gen_<u>mulsidi3_64bit_hilo (hilo, operands[1], > operands[2])); Here too just ISA_HAS_DMULT. Several other cases later on, I won't bore you with them all :-) > @@ -2537,7 +2541,7 @@ > (set (match_operand:GPR 3 "register_operand") > (mod:GPR (match_dup 1) > (match_dup 2)))] > - "!TARGET_FIX_VR4120" > + "!TARGET_FIX_VR4120 && TARGET_HAS_<D>DIV" > { > if (TARGET_MIPS16) > { Would prefer the ISA_HAS_<D>DIV first. Please update the MIPS16 patterns in the same way. > @@ -1881,11 +1881,17 @@ mipsisa64sb1-*-elf* | mipsisa64sb1el-*-e > target_cpu_default="MASK_64BIT|MASK_FLOAT64" > tm_defines="${tm_defines} MIPS_ISA_DEFAULT=64 > MIPS_CPU_STRING_DEFAULT=\\\"sb1\\\" MIPS_ABI_DEFAULT=ABI_O64" > ;; > -mips-*-elf* | mipsel-*-elf*) > +mips-*-elf* | mipsel-*-elf* | mipsr5900-*-elf* | mipsr5900el-*-elf*) > tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h" > tmake_file="mips/t-elf" > ;; > -mips64-*-elf* | mips64el-*-elf*) > +mips64r5900-*-elf* | mips64r5900el-*-elf*) > + tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h" > + tmake_file="mips/t-elf" > + target_cpu_default="MASK_64BIT" > + tm_defines="${tm_defines} MIPS_ISA_DEFAULT=3 MIPS_ABI_DEFAULT=ABI_N32" > + ;; > +mips64-*-elf* | mips64el-*-elf* | mips64r5900-*-elf* | mips64r5900el-*-elf*) > tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h" > tmake_file="mips/t-elf" > target_cpu_default="MASK_64BIT|MASK_FLOAT64" The change to the "mips64-*-elf* | mips64el-*-elf*)" line looks unnecessary. > @@ -3374,7 +3400,7 @@ case "${target}" in > supported_defaults="abi arch arch_32 arch_64 float tune tune_32 > tune_64 divide llsc mips-plt synci" > > case ${with_float} in > - "" | soft | hard) > + "" | soft | hard | single | double) > # OK > ;; > *) Please leave this out for now and add it with the ABI changes mentioned above. I can't approve the libgcc bits, but I'm afraid they probably tip the balance against this being acceptable for 4.8. Richard