What happens if reg > 16? Also, the argument reg should be unsigned. If rev > 16 the instruction is invalid. What type of error can/should I throw here.
This does not handle the case xra == XR16. From the doc: I do not see where the case is un-handled. XR16 maps to index 15 in the mxu_gpr array. -----Original Message----- From: Aleksandar Markovic <amarko...@wavecomp.com> Sent: Tuesday, August 28, 2018 10:43 AM To: Janeczek, Craig <jancr...@amazon.com>; qemu-devel@nongnu.org Cc: aurel...@aurel32.net Subject: Re: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I > From: Craig Janeczek <jancr...@amazon.com> > Sent: Tuesday, August 28, 2018 3:00 PM > To: qemu-devel@nongnu.org > Cc: Aleksandar Markovic; aurel...@aurel32.net; Craig Janeczek > Subject: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and > S32M2I > > This commit makes the MXU registers and the utility functions for > reading/writing to them. This is required for full MXU instruction > support. > > Adds support for emulating the S32I2M and S32M2I MXU instructions. > > Signed-off-by: Craig Janeczek <jancr...@amazon.com> > --- > v1 > - initial patch > v2 > - Fix checkpatch.pl errors > - remove mips64 ifdef > - changed bitfield usage to extract32 > - squashed register addition patch into this one > v3 > - Split register addition and opcode enum definition into seperate patches > - Split gen_mxu function into command specific gen_mxu_<ins> > functions > > target/mips/translate.c | 62 > +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/target/mips/translate.c b/target/mips/translate.c index > ae6b16ecd7..f6991aa8ef 100644 > --- a/target/mips/translate.c > +++ b/target/mips/translate.c > @@ -1610,6 +1610,23 @@ static inline void gen_store_gpr (TCGv t, int reg) > tcg_gen_mov_tl(cpu_gpr[reg], t); } > > +/* MXU General purpose registers moves. */ static inline void > +gen_load_mxu_gpr(TCGv t, int reg) { > + if (reg == 0) { > + tcg_gen_movi_tl(t, 0); > + } else { > + tcg_gen_mov_tl(t, mxu_gpr[reg - 1]); > + } > +} What happens if reg > 16? Also, the argument reg should be unsigned. > + > +static inline void gen_store_mxu_gpr(TCGv t, int reg) { > + if (reg != 0) { > + tcg_gen_mov_tl(mxu_gpr[reg - 1], t); > + } > +} > + What happens if reg > 16? Also, the argument reg should be unsigned. > /* Moves to/from shadow registers. */ static inline void > gen_load_srsgpr (int from, int to) { @@ -3798,6 +3815,42 @@ static > void gen_cl (DisasContext *ctx, uint32_t opc, > } > } > > +/* MXU Instructions */ > + > +/* S32I2M XRa, rb - Register move from GRF to XRF */ static void > +gen_mxu_s32i2m(DisasContext *ctx, uint32_t opc) { > + TCGv t0; > + uint32_t xra, rb; > + > + t0 = tcg_temp_new(); > + > + xra = extract32(ctx->opcode, 6, 5); > + rb = extract32(ctx->opcode, 16, 5); > + > + gen_load_gpr(t0, rb); > + gen_store_mxu_gpr(t0, xra); > + > + tcg_temp_free(t0); > +} This does not handle the case xra == XR16. From the doc: "In MXU, a dedicated register file named XRF comprises sixteen 32-bit general purpose registers - XR0~XR15. XR0 is a special one, which always is read as zero. Moreover, XR16 is an alias of MXU_CR described below and it can only be accessed by S32I2M/S32M2I instructions." > + > +/* S32M2I XRa, rb - Register move from XRF to GRF */ static void > +gen_mxu_s32m2i(DisasContext *ctx, uint32_t opc) { > + TCGv t0; > + uint32_t xra, rb; > + > + t0 = tcg_temp_new(); > + > + xra = extract32(ctx->opcode, 6, 5); > + rb = extract32(ctx->opcode, 16, 5); > + > + gen_load_mxu_gpr(t0, xra); > + gen_store_gpr(t0, rb); > + > + tcg_temp_free(t0); > +} > + > /* Godson integer instructions */ > static void gen_loongson_integer(DisasContext *ctx, uint32_t opc, > int rd, int rs, int rt) @@ -17894,6 > +17947,15 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, > DisasContext *ctx) > check_insn(ctx, INSN_LOONGSON2F); > gen_loongson_integer(ctx, op1, rd, rs, rt); > break; > + > + case OPC_MXU_S32I2M: > + gen_mxu_s32i2m(ctx, op1); > + break; > + > + case OPC_MXU_S32M2I: > + gen_mxu_s32m2i(ctx, op1); > + break; > + > case OPC_CLO: > case OPC_CLZ: > check_insn(ctx, ISA_MIPS32);