Re: [PATCH MIPS][LS3A] Add Loongson3A mul/div instructions

[Mingming Sun <mingm dot sun at gmail dot com>]

On Wed, Dec 1, 2010 at 6:16 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Mingming Sun <mingm.sun@gmail.com> writes:
>> 2010-11-29 Mingming Sun <mingm.sun@gmail.com>
>> ? ? ? gas/ChangeLog
>> ? ? ? * config/tc-mips.c (macro_build): Add loongson3a-specific instructions support.
>> ? ? ? (validate_mips_insn): Likewise.
>> ? ? ? (mips_ip): Likewise.
>>
>> ? ? ? include/ChangeLog
>> ? ? ? * opcode/mips.h: Define OP_SH_MINUS_A, OP_MASK_MINUS_A, OP_SH_MINUS_B,
>> ? ? ? OP_MASK_MINUS_B, OP_SH_MINUS_F, OP_MASK_MINUS_F, OP_SH_RY, OP_MASK_RY,
>> ? ? ? OP_SH_FZ, OP_MASK_FZ, INSN_WRITE_GPR_Y, INSN_WRITE_FPR_Z,
>> ? ? ? INSN_READ_GPR_D, INSN_READ_GPR_Y, INSN_READ_FPR_Z.
>>
>> ? ? ? opcodes/ChangeLog
>> ? ? ? * mips-dis.c (print_insn_args): Add loongson3a-specific instructions support.
>> ? ? ? * mips-opc.c: Define WR_z, RD_y, RD_z, RD_d.
>> ? ? ? (mips_builtin_opcodes): Add loongson3a-specific instructions.
>
> There's a big comment in include/opcode/mips.h that explains what
> the format characters mean, and which ones have been used. ?You need
> to update that as well.
>
> Is there any need for a new extension character, "-"? ?Many characters
> in the "+" group are still free, and they aren't set aside for a
> particular purpose. ?I'd also prefer "+Z" over "^" for the FZ fields.
>
>> @@ -430,6 +442,12 @@ struct mips_opcode
>> ?#define INSN_WRITE_FPR_S ? ? ? ? ? ?0x00000010
>> ?/* Modifies the floating point register in OP_*_FT. ?*/
>> ?#define INSN_WRITE_FPR_T ? ? ? ? ? ?0x00000020
>> +/* Loongson-3A: Modifies the general purpose register in OP_*_RY. ?*/
>> +#define INSN_WRITE_GPR_Y ? ? ? ? ? ?0x00000009
>> +/* Loongson-3A: Modifies the floating point register in OP_*_FZ. ?*/
>> +#define INSN_WRITE_FPR_Z ? ? ? ? ? ?0x00000090
>> +/* Loongson-3A: Reads the general purpose register in OP_*_RD. ?*/
>> +#define INSN_READ_GPR_D ? ? ? ? ? ? 0x0000000c
>> ?/* Reads the general purpose register in OP_*_RS. ?*/
>> ?#define INSN_READ_GPR_S ? ? ? ? ? ? 0x00000040
>> ?/* Reads the general purpose register in OP_*_RT. ?*/
>> @@ -440,6 +458,11 @@ struct mips_opcode
>> ?#define INSN_READ_FPR_T ? ? ? ? ? ? 0x00000200
>> ?/* Reads the floating point register in OP_*_FR. ?*/
>> ?#define INSN_READ_FPR_R ? ? ? ? ? ? ? ? ?0x00000400
>> +/* Loongson-3A: Reads the general purpose register in OP_*_RY. ?*/
>> +#define INSN_READ_GPR_Y ? ? ? ? ? ? 0x00000030
>> +/* Loongson-3A: Reads the floating point register in OP_*_FZ. ?*/
>> +#define INSN_READ_FPR_Z ? ? ? ? ? ? 0x00000900
>
> These INSN_*s values are supposed to be bitflags, so (to take an example),
> 0x900 already means "reads the FPR in the FS field and modifies the
> condition codes". ?You should be using new INSN2_* flags instead.
>
> You should also be checking these flags in tc-mips.c, in the same
> way that existing read and write flags are set. ?This controls things
> like automatic delay-slot filling. ?For example, at the moment:
>
> ? ? ? ?gslq $4,$5,0($6)
> ? ? ? ?beq ?$4,$0,1f
> ? ? ? ?nop
> 1:
>
> produces:
>
> ? 0: ? 10800002 ? ? ? ?beqz ? ?a0,0xc
> ? 4: ? c8c50024 ? ? ? ?gslq ? ?a0,a1,0(a2)
> ? 8: ? 00000000 ? ? ? ?nop
>
> which looks wrong.
>
> There needs to be some gas testcases too.
>
> Richard
>

Thanks, Richard.
This new patch only adds mul/div instructions support in opcode/mips-opt.c.
I'm checking other problems you've mentioned.

Thanks,
Mingming Sun



2010-12-01 Mingming Sun <mingm.sun@gmail.com>
	opcodes/
	* mips-opc.c: (mips_builtin_opcodes): Add loongson3a mul/div instructions.

Attachment: patch.txt
Description: Text document