This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] [MIPS] [Loongson] Specific integer instructions and prefetch added
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Ruan Beihong" <ruanbeihong at gmail dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 10 Nov 2008 20:48:25 +0000
- Subject: Re: [PATCH] [MIPS] [Loongson] Specific integer instructions and prefetch added
- References: <b45dfd470811062229u1a8b1012ibe4cf63cc68067ff@mail.gmail.com>
"Ruan Beihong" <ruanbeihong@gmail.com> writes:
> 2008/11/6, Ruan Beihong <ruanbeihong@gmail.com>:
>> Hi Richard,
>> It has been so many days since last talk. I have signed a assignment
>> now (due to my fault that I'd given a wrong address and also the slow
>> mailing service between USA and China), and the assignment is now on
>> its way to Boston for a few days.
>> So can the patch be accept now?
>> I still think that the loongson2ef's <d>multu.g should be divided into
>> a new type of insn (I call it mul3nc), since they don't change or use
>> the HI/LO register.
>>
>> --
>> James Ruan
>>
>
> I've got the reply from FSF. My assignment is completed.
Great! Thanks for going through the process.
> +(define_insn "<u>div<mode>3"
> + [(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")))]
> + "TARGET_LOONGSON_2EF"
> + { return mips_output_division ("<d>div<u>.g\t%0,%1,%2", operands); }
> + [(set_attr "type" "idiv3")
> + (set_attr "mode" "<MODE>")]
> +)
> +
> +(define_insn "<u>mod<mode>3"
> + [(set (match_operand:GPR 0 "register_operand" "=d")
> + (any_mod:GPR
> + (match_operand:GPR 1 "register_operand" "d")
> + (match_operand:GPR 2 "register_operand" "d")))]
> + "TARGET_LOONGSON_2EF"
> + { return mips_output_division ("<d>mod<u>.g\t%0,%1,%2", operands); }
> + [(set_attr "type" "idiv3")
> + (set_attr "mode" "<MODE>")]
As Eric says, these instructions need to be earlyclobber ("=&d" rather
than "=d") because the divisor might be checked for zero after the
division itself.
> @@ -1358,7 +1365,12 @@
> ""
> {
> if (ISA_HAS_<D>MUL3)
> - emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1], operands[2]));
> + {
> + if(TARGET_LOONGSON_2EF)
> + emit_insn(gen_mul<mode>3_mul3_ls2ef(operands[0],operands[1],operands[2]));
> + else
> + emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1], operands[2]));
> + }
> else if (TARGET_FIX_R4000)
> emit_insn (gen_mul<mode>3_r4000 (operands[0], operands[1], operands[2]));
> else
Formatting; this should be:
{
if (TARGET_LOONGSON_2EF)
emit_insn (gen_mul<mode>3_mul3_ls2ef (operands[0], operands[1],
operands[2]));
else
emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1], operands[2]));
}
(GCC's quite strict about formatting.) But I don't see any reason to make
the TARGET_LOONSON_2EF code depend on ISA_HAS_<D>MUL3; it seems easier
to have:
if (TARGET_LOONGSON_2EF)
emit_insn (gen_mul<mode>3_mul3_ls2ef (operands[0], operands[1],
operands[2]));
else if (ISA_HAS_<D>MUL3)
emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1], operands[2]));
else if (TARGET_FIX_R4000)
...
instead.
> +(define_insn "mul<mode>3_mul3_ls2ef"
> + [(set (match_operand:GPR 0 "register_operand" "=d")
> + (mult:GPR (match_operand:GPR 1 "register_operand" "d")
> + (match_operand:GPR 2 "register_operand" "d")))]
> + "TARGET_LOONGSON_2EF"
> +{
> + return "<d>multu.g\t%0,%1,%2";
> +}
No need for C code here. Just use:
(define_insn "mul<mode>3_mul3_ls2ef"
[(set (match_operand:GPR 0 "register_operand" "=d")
(mult:GPR (match_operand:GPR 1 "register_operand" "d")
(match_operand:GPR 2 "register_operand" "d")))]
"TARGET_LOONGSON_2EF"
"<d>multu.g\t%0,%1,%2"
> +(define_peephole2
> + [(set (match_operand:SI 0 "lo_operand")
> + (mult:SI (match_operand:SI 1 "d_operand")
> + (match_operand:SI 2 "d_operand")))
> + (set (match_operand:SI 3 "d_operand")
> + (match_dup 0))]
> + "TARGET_LOONGSON_2EF && peep2_reg_dead_p(2, operands[0])"
> + [(set (match_dup 3)
> + (mult:SI (match_dup 1)
> + (match_dup 2)))]
When do we need this? How do we get a LO-target multiplication
after the mul<mode>3 change above?
> @@ -6128,6 +6163,9 @@
> (match_operand 2 "const_int_operand" "n"))]
> "ISA_HAS_PREFETCH && TARGET_EXPLICIT_RELOCS"
> {
> + if(TARGET_LOONGSON_2EF)
> + /*Loongson 2[ef] use load to $0 to perform prefetching*/
> + return "ld\t$0,%a0";
Formatting; should be:
if (TARGET_LOONGSON_2EF)
/* Loongson 2[ef] use load to $0 to perform prefetching. */
return "ld\t$0,%a0";
> /* ISA has a three-operand multiplication instruction. */
> #define ISA_HAS_DMUL3 (TARGET_64BIT \
> - && TARGET_OCTEON \
> + && (TARGET_OCTEON \
> + || TARGET_LOONGSON_2EF) \
> && !TARGET_MIPS16)
Not needed after the change I suggested above.
Looks good otherwise, thanks.
Richard