Message ID | cover.1540134918.git.noring@nocrew.org |
---|---|
Headers | show |
Series | target/mips: Limited support for the R5900 | expand |
> From: Fredrik Noring <noring@nocrew.org> > Subject: [PATCH v8 00/38] target/mips: Limited support for the R5900 > Hi, Fredrik. The series looks good. I added ASE_MMI flag along with INSN_R5900, I think this fits better in the overall MIPS for QEMU design. I made a couple of other minor changes. I experienced some build errors (see the end of this mail), so I had to exclude some patches, but all others are fine, and had my "Reviewed-by". 32 patches will be included in the next MIPS queue. Thanks for all efforts! Aleksandar /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c: In function ‘gen_mul_txx9’: /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4888:38: error: passing argument 3 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4888:51: error: passing argument 4 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4908:38: error: passing argument 3 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:4908:51: error: passing argument 4 of ‘tcg_gen_add2_i32’ from incompatible pointer type [-Werror=incompatible-pointer-types] tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); ^~~~~~ In file included from /home/rtrk/Build/qemu-pull-request-oct-22/target/mips/translate.c:29:0: /home/rtrk/Build/qemu-pull-request-oct-22/tcg/tcg-op.h:314:6: note: expected ‘TCGv_i32 {aka struct TCGv_i32_d *}’ but argument is of type ‘TCGv_i64 {aka struct TCGv_i64_d *}’ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al, ^~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors /home/rtrk/Build/qemu-pull-request-oct-22/rules.mak:69: recipe for target 'target/mips/translate.o' failed make[1]: *** [target/mips/translate.o] Error 1 Makefile:483: recipe for target 'subdir-mips64-softmmu' failed make: *** [subdir-mips64-softmmu] Error 2
Many thanks, Aleksandar, > I added ASE_MMI flag along with INSN_R5900, I think this fits better in > the overall MIPS for QEMU design. Maciej -- can we add "MMI" under "ASEs implemented" in the kernel too, even if it is a vendor-specific architecture extension that normally isn't counted as an ASE? QEMU simply calls these "vendor specific ASEs". Aleksandar -- please or ASE_MMI to insn_flags here: --- a/target/mips/translate_init.inc.c +++ b/target/mips/translate_init.inc.c @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = #endif /* !CONFIG_USER_ONLY */ .SEGBITS = 32, .PABITS = 32, - .insn_flags = CPU_R5900, + .insn_flags = CPU_R5900 | ASE_MMI, .mmu_type = MMU_TYPE_R4000, }, { Strictly speaking, MADD, MADDU, MULT, MULTU, MULT1, MULTU1, DIV1, DIVU1, MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1 are not part of what the Toshiba TX System RISC TX79 Core Architecture manual specifies as "Multimedia Instructions", section B.3.2, on page B-3, even though their opcodes are covered by TX79_CLASS_MMI and the decode_tx79_mmi function. Can we adjust ASE_MMI for QEMU accordingly? Also, doesn't it make sense to cover LQ and SQ with ASE_MMI as well, as those two really are MMIs? Finally, as far as I know, the MMIs cannot be disabled on R5900 hardware. --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -26099,7 +26099,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) } break; case OPC_SPECIAL3: - if (ctx->insn_flags & INSN_R5900) { + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { decode_tx79_sq(env, ctx); /* TX79_SQ */ } else { decode_opc_special3(env, ctx); @@ -26763,7 +26763,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) } break; case OPC_MSA: /* OPC_MDMX */ - if (ctx->insn_flags & INSN_R5900) { + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { decode_tx79_lq(env, ctx); /* TX79_LQ */ } else { /* MDMX: Not implemented. */ > I experienced some build errors (see the end of this mail), so I had to > exclude some patches, but all others are fine, and had my "Reviewed-by". > 32 patches will be included in the next MIPS queue. Ah, I didn't test the 64-bit build on the MADD[U][1] instructions. I will look into them and post updated patches. Regarding the R5900 FPU: It appears reasonable to introduce an ELF ABI variant for the nonstandard R5900 FPU. A testsuite covering the anomalies seems to be needed as well. Careful verification on hardware is needed. I think it's probably best to keep the R5900 FPU disabled in QEMU until these things have been sorted out. I discovered that I lost the disassembly of MULT1 and MULTU1 in v8, as shown in the attached patch below. This small change belongs to commit bebf09ef3977 ("target/mips: Support R5900 three-operand MULT1 and MULTU1 instructions") in your tags/mips-queue-oct-2018-part-2. Please apply: --- a/disas/mips.c +++ b/disas/mips.c @@ -2736,10 +2736,14 @@ const struct mips_opcode mips_builtin_opcodes[] = {"mult", "s,t", 0x00000018, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, I1 }, {"mult", "7,s,t", 0x00000018, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, D33 }, {"mult", "d,s,t", 0x00000018, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, 0, G1 }, +{"mult1", "s,t", 0x70000018, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, +{"mult1", "d,s,t", 0x70000018, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, EE }, {"multp", "s,t", 0x00000459, 0xfc00ffff, RD_s|RD_t|MOD_HILO, 0, SMT }, {"multu", "s,t", 0x00000019, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, I1 }, {"multu", "7,s,t", 0x00000019, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, D33 }, {"multu", "d,s,t", 0x00000019, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, 0, G1 }, +{"multu1", "s,t", 0x70000019, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, +{"multu1", "d,s,t", 0x70000019, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, EE }, {"mulu", "d,s,t", 0x00000059, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d, 0, N5 }, {"neg", "d,w", 0x00000022, 0xffe007ff, WR_d|RD_t, 0, I1 }, /* sub 0 */ {"negu", "d,w", 0x00000023, 0xffe007ff, WR_d|RD_t, 0, I1 }, /* subu 0 */ Fredrik
> From: Fredrik Noring <noring@nocrew.org> > > Subject: Re: [PATCH v8 00/38] target/mips: Limited support for the R5900 > > Many thanks, Aleksandar, > > > I added ASE_MMI flag along with INSN_R5900, I think this fits better in > > the overall MIPS for QEMU design. > > Maciej -- can we add "MMI" under "ASEs implemented" in the kernel too, > even if it is a vendor-specific architecture extension that normally > isn't counted as an ASE? QEMU simply calls these "vendor specific ASEs". > > Aleksandar -- please or ASE_MMI to insn_flags here: > > --- a/target/mips/translate_init.inc.c > +++ b/target/mips/translate_init.inc.c > @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = > #endif /* !CONFIG_USER_ONLY */ > .SEGBITS = 32, > .PABITS = 32, > - .insn_flags = CPU_R5900, > + .insn_flags = CPU_R5900 | ASE_MMI, > .mmu_type = MMU_TYPE_R4000, > }, > { > Hi, Fredrik. I understood what you said about ASE_MMI and other changes you want to be included. Pull request with 32 patches from this series is already sent, and I would like to avoid sending v2 of that request. Let's wait for some time until the pull request is hopefully accepted. There will be most likely another one at the beginning of the next week. We are appoaching QEMU 3.1 soft freeze (Oct 30), and at this point we would like to stabilize the code, and to integrate only crucial patches. I suggest that you create a new series "target/mips: Amend R5900 support". It should be based on the code submitted in the pull request. Place the most crucial patches as the first ones, at the beginning of the series. Less important at the end. FPU changes are too risky at this stage od 3.1 development cycle, and I would leave them for QEMU 3.2+. Regards and thanks again, Aleksandar > Strictly speaking, MADD, MADDU, MULT, MULTU, MULT1, MULTU1, DIV1, DIVU1, > MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1 are not part of what the > Toshiba TX System RISC TX79 Core Architecture manual specifies as > "Multimedia Instructions", section B.3.2, on page B-3, even though > their opcodes are covered by TX79_CLASS_MMI and the decode_tx79_mmi > function. Can we adjust ASE_MMI for QEMU accordingly? > > Also, doesn't it make sense to cover LQ and SQ with ASE_MMI as well, as > those two really are MMIs? > > Finally, as far as I know, the MMIs cannot be disabled on R5900 hardware. > > --- a/target/mips/translate.c > +++ b/target/mips/translate.c > @@ -26099,7 +26099,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) > } > break; > case OPC_SPECIAL3: > - if (ctx->insn_flags & INSN_R5900) { > + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { > decode_tx79_sq(env, ctx); /* TX79_SQ */ > } else { > decode_opc_special3(env, ctx); > @@ -26763,7 +26763,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) > } > break; > case OPC_MSA: /* OPC_MDMX */ > - if (ctx->insn_flags & INSN_R5900) { > + if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { > decode_tx79_lq(env, ctx); /* TX79_LQ */ > } else { > /* MDMX: Not implemented. */ > > > I experienced some build errors (see the end of this mail), so I had to > > exclude some patches, but all others are fine, and had my "Reviewed-by". > > 32 patches will be included in the next MIPS queue. > > Ah, I didn't test the 64-bit build on the MADD[U][1] instructions. I will > look into them and post updated patches. > > Regarding the R5900 FPU: It appears reasonable to introduce an ELF ABI > variant for the nonstandard R5900 FPU. A testsuite covering the anomalies > seems to be needed as well. Careful verification on hardware is needed. > I think it's probably best to keep the R5900 FPU disabled in QEMU until > these things have been sorted out. > > I discovered that I lost the disassembly of MULT1 and MULTU1 in v8, as > shown in the attached patch below. This small change belongs to commit > bebf09ef3977 ("target/mips: Support R5900 three-operand MULT1 and MULTU1 > instructions") in your tags/mips-queue-oct-2018-part-2. Please apply: > > --- a/disas/mips.c > +++ b/disas/mips.c > @@ -2736,10 +2736,14 @@ const struct mips_opcode mips_builtin_opcodes[] = > {"mult", "s,t", 0x00000018, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, > I1 }, > {"mult", "7,s,t", 0x00000018, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, > D33 }, > {"mult", "d,s,t", 0x00000018, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, > 0, G1 }, > +{"mult1", "s,t", 0x70000018, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, > +{"mult1", "d,s,t", 0x70000018, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, > EE }, > {"multp", "s,t", 0x00000459, 0xfc00ffff, RD_s|RD_t|MOD_HILO, 0, > SMT }, > {"multu", "s,t", 0x00000019, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0, > I1 }, > {"multu", "7,s,t", 0x00000019, 0xfc00e7ff, WR_a|RD_s|RD_t, 0, > D33 }, > {"multu", "d,s,t", 0x00000019, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, > 0, G1 }, > +{"multu1", "s,t", 0x70000019, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE }, > +{"multu1", "d,s,t", 0x70000019, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, > EE }, > {"mulu", "d,s,t", 0x00000059, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d, 0, > N5 }, > {"neg", "d,w", 0x00000022, 0xffe007ff, WR_d|RD_t, 0, > I1 }, /* sub 0 */ > {"negu", "d,w", 0x00000023, 0xffe007ff, WR_d|RD_t, 0, > I1 }, /* subu 0 */ > > Fredrik >
Hi Maciej, > > I added ASE_MMI flag along with INSN_R5900, I think this fits better in > > the overall MIPS for QEMU design. > > Maciej -- can we add "MMI" under "ASEs implemented" in the kernel too, > even if it is a vendor-specific architecture extension that normally > isn't counted as an ASE? QEMU simply calls these "vendor specific ASEs". I have no authority to approve such a change for the kernel, but it looks reasonable to me and I will support you with it, with one reservation however. As this is an ISA extension in the vendor-specific space, I think it belongs to a vendor-specific namespace, so as to make it clear it is not a generic architectural feature and also to avoid name clashes. So it has to be called Toshiba MMI or suchlike, similarly to how I requested that for the Longsoon MMI feature in a recent binutils review (cf <https://sourceware.org/ml/binutils/2018-07/msg00201.html> and binutils commit 8095d2f70e1a ("MIPS/GAS: Split Loongson MMI Instructions from loongson2f/3a")), with all the consequences throughout. > Aleksandar -- please or ASE_MMI to insn_flags here: > > --- a/target/mips/translate_init.inc.c > +++ b/target/mips/translate_init.inc.c > @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = > #endif /* !CONFIG_USER_ONLY */ > .SEGBITS = 32, > .PABITS = 32, > - .insn_flags = CPU_R5900, > + .insn_flags = CPU_R5900 | ASE_MMI, > .mmu_type = MMU_TYPE_R4000, > }, > { So I think it better be called ASE_TOSHIBA_MMI here. > Strictly speaking, MADD, MADDU, MULT, MULTU, MULT1, MULTU1, DIV1, DIVU1, > MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and MTLO1 are not part of what the > Toshiba TX System RISC TX79 Core Architecture manual specifies as > "Multimedia Instructions", section B.3.2, on page B-3, even though > their opcodes are covered by TX79_CLASS_MMI and the decode_tx79_mmi > function. Can we adjust ASE_MMI for QEMU accordingly? NB all but pipeline 1 instructions of these are also implemented by other members of the TXx9 family. They seem to be referred to as just "multiply and multiply-add instructions" in the TX79 manual (cf Section B.3.1). > Also, doesn't it make sense to cover LQ and SQ with ASE_MMI as well, as > those two really are MMIs? And they're certainly listed as such in the TX79 manual (cf Section B.3.2). > Regarding the R5900 FPU: It appears reasonable to introduce an ELF ABI > variant for the nonstandard R5900 FPU. Indeed and in particular given that the R5900 does not produce any FPU exceptions it should be quite straightforward for the Linux kernel to recognise this specific ABI annotation with ELF binaries and switch its FP environment between R5900 native float and IEEE 754 emulated float accordingly. We could then make QEMU run in the user emulation mode do the same. Of course all the pieces of the toolchain as well as the dynamic loader in use would have to taught to prevent incompatible pieces of hard float code from being used together. Maciej
On Mon, 22 Oct 2018, Maciej W. Rozycki wrote:
> Hi Maciej,
What an odd copy & paste thinko! I can't believe I addressed myself in
the opening of my e-mail. :)
Maciej
Hi Aleksandar, > Pull request with 32 patches from this series is already sent, and I would > like to avoid sending v2 of that request. Let's wait for some time until > the pull request is hopefully accepted. There will be most likely another > one at the beginning of the next week. > > We are appoaching QEMU 3.1 soft freeze (Oct 30), and at this point we > would like to stabilize the code, and to integrate only crucial patches. > I suggest that you create a new series "target/mips: Amend R5900 support". > It should be based on the code submitted in the pull request. Place the > most crucial patches as the first ones, at the beginning of the series. The R5900 testsuite in tests/tcg/mips/mipsr5900 fails unless ASE_MMI is part of insn_flags for the R5900: --- a/target/mips/translate_init.inc.c +++ b/target/mips/translate_init.inc.c @@ -466,7 +466,7 @@ const mips_def_t mips_defs[] = #endif /* !CONFIG_USER_ONLY */ .SEGBITS = 32, .PABITS = 32, - .insn_flags = CPU_R5900, + .insn_flags = CPU_R5900 | ASE_MMI, .mmu_type = MMU_TYPE_R4000, }, { Perhaps that is the only (somewhat) crucial problem, depending on how the testsuites are used, of course. The other ASE_MMI changes and the disassembly of MULT1 and MULTU1 can wait, as can R5900 support for MADD, MADDU, MADD1 and MADDU1, in my opinion. > FPU changes are too risky at this stage od 3.1 development cycle, and I > would leave them for QEMU 3.2+. Agreed! As Maciej just noted, there are also toolchain issues that need to be addressed for the R5900 FPU. Fredrik
On 22/10/18 20:40, Maciej W. Rozycki wrote: > On Mon, 22 Oct 2018, Maciej W. Rozycki wrote: > >> Hi Maciej, > > What an odd copy & paste thinko! I can't believe I addressed myself in > the opening of my e-mail. :) Haha when I saw your mail I thought "weird, there is another Maciej involved in this MIPS specific thread!?"
On Mon, Oct 22, 2018 at 3:34 PM Aleksandar Markovic <amarkovic@wavecomp.com> wrote: > > From: Fredrik Noring <noring@nocrew.org> > > Subject: [PATCH v8 00/38] target/mips: Limited support for the R5900 > > > I experienced some build errors (see the end of this mail), so I had to exclude some patches, but all others are fine, and had my "Reviewed-by". 32 patches will be included in the next MIPS queue. Thank you a lot Aleksandar for taking this series! I appreciate a lot, being backed by a company, you also care about hobbyist contributions. This is not always an easy task for maintainers, and from the hobbyist point of view, this means a lot to me. Regards, Phil.
Hi Maciej, > I have no authority to approve such a change for the kernel, but it looks > reasonable to me and I will support you with it, with one reservation > however. As this is an ISA extension in the vendor-specific space, I > think it belongs to a vendor-specific namespace, so as to make it clear it > is not a generic architectural feature and also to avoid name clashes. > > So it has to be called Toshiba MMI or suchlike, similarly to how I > requested that for the Longsoon MMI feature in a recent binutils review > (cf <https://sourceware.org/ml/binutils/2018-07/msg00201.html> and > binutils commit 8095d2f70e1a ("MIPS/GAS: Split Loongson MMI Instructions > from loongson2f/3a")), with all the consequences throughout. Vendor ASE namespaces makes sense to me. I can prepare a patch for it. > NB all but pipeline 1 instructions of these are also implemented by other > members of the TXx9 family. They seem to be referred to as just "multiply > and multiply-add instructions" in the TX79 manual (cf Section B.3.1). Would ASE_TOSHIBA_MMI -- TX79 128-bit multimedia instructions ASE_TOSHIBA_MAC -- TXx9 multiply and multiply-add instructions (MADD etc.) ASE_TOSHIBA_MAC1 -- TX79 pipeline 1 variant of ASE_TOSHIBA_MAC ASE_TOSHIBA_FMA -- R5900 FPU extensions (MADD.s etc.) be acceptable for the currently known Toshiba extensions? (Please propose better names.) One complication is that it seems only 8 bits are available for all vendor ASEs, and Toshiba would then scoop up half of those. Fredrik
Hi Maciej, > target/mips/translate.c:4888:38: error: passing argument 3 of > ‘tcg_gen_add2_i32’ from incompatible pointer type > [-Werror=incompatible-pointer-types] > tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); > ^~~~~~ Would you know if any MIPS ISA have LO and HI registers that are not 32-bit? In QEMU they can obviously be either 32-bit or 64-bit, which causes the compilation error here. Fredrik
Hi Fredrik, > > target/mips/translate.c:4888:38: error: passing argument 3 of > > ‘tcg_gen_add2_i32’ from incompatible pointer type > > [-Werror=incompatible-pointer-types] > > tcg_gen_add2_i32(t2, t3, cpu_LO[acc], cpu_HI[acc], t2, t3); > > ^~~~~~ > > Would you know if any MIPS ISA have LO and HI registers that are not > 32-bit? In QEMU they can obviously be either 32-bit or 64-bit, which > causes the compilation error here. Actually with all 64-bit MIPS ISAs HI/LO are a pair of 64-bit registers, that is with MIPS III, MIPS IV, and then MIPS64 R1 to R5 ISAs (base R6 ISA removed the MD accumulator, although it has been retained along with the 3 other ones in the DSP ASE). The R5900 CPU is an oddball here, having no 64-bit multiply or divide instructions, however documentation indicates these registers are still 64-bit as far as the base instruction set is concerned, i.e. it says you can actually write the upper halves with any bit patterns explicitly with the MTHI and MTLO instructions. And then they're really 128-bit as far as the full instruction set of the R5900 is concerned, for all the pipeline 1 MD instructions operate on bits 95:64 and some MMI instructions operate on the full 128-bit width of the accumulator. Interestingly enough architecturally trying to use HI/LO values that are not properly sign-extended 32-bit numbers does not make the operation of 32-bit multiply-accumulate instructions unpredictable, as they are specified to simply ignore the upper 32 bits of a 64-bit value contained there, and the the TX79 manual follows. This is unlike with the GPR inputs to all MD instructions, which architecturally have to be sign-extended. Contrariwise, the TX79 manual says that GPR inputs to the unsigned variants of MD instructions have to be zero-extended, and I do hope this is just an editorial mistake and hardware does not follow (especially as the description of MULTU on page A-87 disagress in this regard with one on page B-25, and all the relevant pseudocode operation specifications consistently use NotWordValue as the input validation condition, although that has been nowhere actually formally defined). Otherwise lots of software would break and you'd have to use a DSLL32/DSRL32 instruction pair every time before feeding the result of other 32-bit calculations to those instructions. BTW, notice that the pseudocode operation specification of the TX79 MD instructions does clearly indicate the sign-extension of output HI/LO contents, e.g. for MULTU we have: prod <- (0 || GPR[rs]31..0) * (0 || GPR[rt]31..0) LO63..0 <- (prod 31)32 || prod31..0 HI63..0 <- (prod 63)32 || prod63..32 GPR[rd]63..0 <- (prod 31)32 || prod31..0 HTH, Maciej
Hi Fredrik, > > NB all but pipeline 1 instructions of these are also implemented by other > > members of the TXx9 family. They seem to be referred to as just "multiply > > and multiply-add instructions" in the TX79 manual (cf Section B.3.1). > > Would > > ASE_TOSHIBA_MMI -- TX79 128-bit multimedia instructions > ASE_TOSHIBA_MAC -- TXx9 multiply and multiply-add instructions (MADD etc.) > ASE_TOSHIBA_MAC1 -- TX79 pipeline 1 variant of ASE_TOSHIBA_MAC > ASE_TOSHIBA_FMA -- R5900 FPU extensions (MADD.s etc.) > > be acceptable for the currently known Toshiba extensions? (Please propose > better names.) One complication is that it seems only 8 bits are available > for all vendor ASEs, and Toshiba would then scoop up half of those. I'm not sure if every single random vendor-specific instruction (or a bunch of) deserves its own ASE designation, be it internal or externally exposed. I think the MMI set being a substantial architectural feature makes sense to be shown in /proc/cpuinfo (in Linux), but I don't think there's much more about it. It's limited to 2 implementations only, so internally I think it can well be handled with a macro or static inline function (as appropriate) which boil down to (CPU_R5900 || CPU_TX79). And if you run out of bits for ASEs regardless, then I suggest just to expand the field in question. In QEMU you can rely on the presence of the `uint64_t' data type, so with only 8 bits exhausted you're far from getting into trouble. Maciej
Hi Maciej, > I'm not sure if every single random vendor-specific instruction (or a > bunch of) deserves its own ASE designation, be it internal or externally > exposed. I think the MMI set being a substantial architectural feature > makes sense to be shown in /proc/cpuinfo (in Linux), but I don't think > there's much more about it. It's limited to 2 implementations only, so > internally I think it can well be handled with a macro or static inline > function (as appropriate) which boil down to (CPU_R5900 || CPU_TX79). Are there benefits in leaving out features? Their utility, such as in choosing compiler options, may not correlate with their (lack of) architectural weight. A random pc, for instance, comes fully dressed flying the flags of fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx lahf_lm epb kaiser tpr_shadow vnmi flexpriority ept vpid xsaveopt dtherm ida arat pln pts in its /proc/cpuinfo. It also has a bugs field with cpu_meltdown spectre_v1 spectre_v2 where the R5900 could have an entry for its short loop bug. > And if you run out of bits for ASEs regardless, then I suggest just to > expand the field in question. In QEMU you can rely on the presence of the > `uint64_t' data type, so with only 8 bits exhausted you're far from > getting into trouble. DisasContext::insn_flags is already uint64_t, where bits 63..56 are reserved for vendor-specific ASEs. Of course, one could organise them differently, especially since they may be mutually exclusive, or one could use a new ASE-specific field for them. Fredrik