On 07/19/2018 05:54 AM, Stefan Markovic wrote:
> From: Yongbok Kim <yongbok....@mips.com>
>
> Add emulation of misc nanoMIPS 16-bit instructions from instruction
> pools P16, P16.BR, P16.BRI, P16.4X4 and other related pools.
>
> Signed-off-by: Yongbok Kim <yongbok....@mips.com>
> Signed-off-by: Aleksandar Markovic <amarko...@wavecomp.com>
> Signed-off-by: Stefan Markovic <smarko...@wavecomp.com>
> ---
> target/mips/translate.c | 258
> ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 258 insertions(+)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 4e6ae1f..798f977 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -16502,6 +16502,264 @@ static int decode_gpr_gpr4_zero(int r)
>
> static int decode_nanomips_opc(CPUMIPSState *env, DisasContext *ctx)
> {
> + uint32_t op;
> + int rt = decode_gpr_gpr3(NANOMIPS_EXTRACT_RD(ctx->opcode));
> + int rs = decode_gpr_gpr3(NANOMIPS_EXTRACT_RS(ctx->opcode));
> + int rd = decode_gpr_gpr3(NANOMIPS_EXTRACT_RS1(ctx->opcode));
> +
> + /* make sure instructions are on a halfword boundary */
> + if (ctx->base.pc_next & 0x1) {
> + env->CP0_BadVAddr = ctx->base.pc_next;
You can't assign to ENV here. You must generate code to do this:
TCGv tmp = tcg_const_tl(ctx->base.pc_next);
tcg_gen_st_tl(tmp, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));
tcg_temp_free(tmp);
> + generate_exception_end(ctx, EXCP_AdEL);
> + return 2;
> + }
> +
> + op = (ctx->opcode >> 10) & 0x3f;
> + switch (op) {
> + case NM_P16_MV:
> + {
> + int rt = NANOMIPS_EXTRACT_RD5(ctx->opcode);
> + if (rt != 0) {
> + /* MOVE */
> + int rs = NANOMIPS_EXTRACT_RS5(ctx->opcode);
> + gen_arith(ctx, OPC_ADDU, rt, rs, 0);
If you've any thought for nanoMIPS64 in future, consider using OR instead.
> + case NM_ADDIUR2:
> + {
> + uint8_t u = (uint8_t) extract32(ctx->opcode, 0, 3) << 2;
> + gen_arith_imm(ctx, OPC_ADDIU, rt, rs, u);
Drop the useless cast? And the variable, come to that; just above you extract
the immediate to gen_arith_imm as an argument.
> + }
> + break;
> + case NM_P_ADDIURS5:
> + {
> + int rt = extract32(ctx->opcode, 5, 5);
This shadows the outer rt variable. Just overwrite the original.
> + if (rt != 0) {
> + int s = (sextract32(ctx->opcode, 4, 1) << 3) |
> + extract32(ctx->opcode, 0, 3);
> + /* s = sign_extend( s[3] . s[2:0] , from_nbits = 4)*/
> + gen_arith_imm(ctx, OPC_ADDIU, rt, rt, s);
> + }
> + }
> + break;
> + }
> + break;
There's some really bad indentation going on here.
> + case NM_P16_4X4:
> + {
> + int rt = (extract32(ctx->opcode, 9, 1) << 3) |
> + extract32(ctx->opcode, 5, 3);
> + int rs = (extract32(ctx->opcode, 4, 1) << 3) |
> + extract32(ctx->opcode, 0, 3);
Shadowing again.
> + default:
> + /* P16.BRI */
> + if (extract32(ctx->opcode, 4, 3) < extract32(ctx->opcode, 7, 3))
> {
These are already extracted into rs and rt...
> + /* BEQC16 */
> + gen_compute_branch(ctx, OPC_BEQ, 2, rs, rt,
> + extract32(ctx->opcode, 0, 4) << 1, 0);
... which you are using here.
And surely, merge these as
gen_compute_branch(ctx, rs < rt ? OPC_BEQ : OPC_BNE, 2, rs, rt,
extract32(ctx->opcode, 0, 4) << 1, 0);
r~
