[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-ope
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU |
Date: |
Sun, 14 Oct 2018 23:56:17 +0200 |
Hi Fredrik,
On Sun, Oct 14, 2018 at 6:41 PM Fredrik Noring <address@hidden> wrote:
>
> Hi Philippe,
>
> > --- a/target/mips/translate.c
> > +++ b/target/mips/translate.c
> > @@ -3843,6 +3843,46 @@ static void gen_mul_txx9(DisasContext *ctx, uint32_t
> > opc,
>
> What about documenting MADD and MADDU along with MULT and MULTU in the
> note above?
OK.
>
> > + case OPC_MADD:
>
> This case is unreachable, because gen_mul_txx9 will never be called for
> OPC_MADD.
Well... I was going to send my R3900 branch but you sent your R5900
branch which clashes with mine for a single #define.
I posted a series to avoid this clash [*]: "mips: Increase the
insn_flags holder size and clean mips-defs.h" so we could both work
together.
Unfortunately Aleksandar prefer I don't base my series on top of
yours, so I'm blocked waiting your to enter.
I did rebase mine on yours to avoid duplicated efforts, and your
series was easier to review, less than 10 patchs.
Since I'm modelling a SoC (QEMU system part) mine is quite bigger.
I then realized the 3.0 merge window is closing (so I now have to wait
for the 3.1 to open in January), and I might have less time to spend
on this.
So I started to look at what patches I could salvage (a patch open
posted on the mailing list is still better than a phantom one IMHO,
even if WIP or invalid).
That's true it is not reachable, it lacks the INSN_R3900 definition,
used by the R3900 mips_def_t.
I'll stop bothering with this until the code is reachable (my branch posted).
[*] https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg04072.html
>
> > + TCGv_i64 t2 = tcg_temp_new_i64();
> > + TCGv_i64 t3 = tcg_temp_new_i64();
>
> The MADD (and MADDU) instructions are defined to multiply 32-bit integers
> in the C790 manual. Are 64-bit integers required to perform this with QEMU?
tcg_gen_mul_i32() store the multiplication result in a 32bit register.
If you look at "Table 3-8. Multiply, multiply / add instructions
(R3000A extended instruction set)":
MADD rd, rs, rt
MADD rs, rt
Multiply the contents of registers rs and rt as two’s complement
integers, and add the doubleword
(64-bit) result to multiply/divide registers HI and LO. Also, store
the lower 32 bits of the add result
in register rd. In the MADD rs, rt format, the store operation to a
general register is omitted.
To be able to use the 64-bit result we need to use tcg_gen_mul_i64().
> > + gen_move_low32(cpu_LO[acc], t2);
> > + gen_move_high32(cpu_HI[acc], t2);
> > + if (rd) {
> > + gen_move_low32(cpu_gpr[rd], t2);
>
> Are LO, HI and GPR[rd] sign-extended to 64 bits when required?
tcg_gen_mul_i64() seems to use unsigned internally (calling
tcg_gen_mulu2_i32()).
So this one might be wrong.
(Richard can you confirm?)
>
> > + case OPC_MADDU:
>
> As above, this case is unreachable, because gen_mul_txx9 will never be
> called for OPC_MADDU.
>
> > + gen_move_low32(cpu_LO[acc], t2);
> > + gen_move_high32(cpu_HI[acc], t2);
> > + if (rd) {
> > + gen_move_low32(cpu_gpr[rd], t2);
>
> As above, are LO, HI and GPR[rd] sign-extended to 64 bits when required?
MADDU rd, rs, rt
MADDU rs, rt
Multiply the contents of registers rs and rt as unsigned integers,
and add the doubleword (64-bit)
result to multiply/divide registers HI and LO. Also, store the lower
32 bits of the add result in register
rd. In the MADDU rs, rt format, the store operation to a general
register is omitted.
This one looks correct.
>
> Fredrik
Thanks for your review,
Phil.
- [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Philippe Mathieu-Daudé, 2018/10/14
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Fredrik Noring, 2018/10/14
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU,
Philippe Mathieu-Daudé <=
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Maciej W. Rozycki, 2018/10/14
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Fredrik Noring, 2018/10/15
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Aleksandar Markovic, 2018/10/16
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Fredrik Noring, 2018/10/16
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Richard Henderson, 2018/10/16
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Fredrik Noring, 2018/10/16
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Maciej W. Rozycki, 2018/10/16
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Aleksandar Markovic, 2018/10/19
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Aleksandar Markovic, 2018/10/28
- Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU, Maciej W. Rozycki, 2018/10/28