[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] target-mips: fix broken MIPS16 and microMIPS
From: |
Leon Alrae |
Subject: |
Re: [Qemu-devel] [PATCH v2] target-mips: fix broken MIPS16 and microMIPS |
Date: |
Mon, 22 Sep 2014 14:55:01 +0100 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
Hi Yongbok,
For me this patch looks fine, just minor nitpicks:
> if (blink > 0) {
> int post_delay = insn_bytes;
> int lowbit = !!(ctx->hflags & MIPS_HFLAG_M16);
>
> - if (opc != OPC_JALRC)
> - post_delay += ((ctx->hflags & MIPS_HFLAG_BDS16) ? 2 : 4);
> -
> + post_delay += delayslot_size;
Wouldn't it be better to remove this line add delayslot_size during
post_delay initialization?
> + if (ctx->hflags & MIPS_HFLAG_BDS_STRICT) {
> + switch (op & 0x7) { /* MSB-3..MSB-5 */
> + case 0:
> + /* POOL31A, POOL32B, POOL32I, POOL32C */
POOL32A :)
> case OPC_J ... OPC_JAL: /* Jump */
> offset = (int32_t)(ctx->opcode & 0x3FFFFFF) << 2;
> - gen_compute_branch(ctx, op, 4, rs, rt, offset);
> + gen_compute_branch(ctx, op, 4, rs, rt, offset, 4);
> break;
> case OPC_BEQ ... OPC_BGTZ: /* Branch */
> case OPC_BEQL ... OPC_BGTZL:
> - gen_compute_branch(ctx, op, 4, rs, rt, imm << 2);
> + gen_compute_branch(ctx, op, 4, rs, rt, imm << 2, 4);
Indentation issue?
> @@ -15719,6 +15648,13 @@ gen_intermediate_code_internal(MIPSCPU *cpu,
> TranslationBlock *tb,
> ctx.bstate = BS_STOP;
> break;
> }
> + if (ctx.hflags & MIPS_HFLAG_BMASK) {
> + if (!(ctx.hflags & MIPS_HFLAG_BDS16) &&
> + !(ctx.hflags & MIPS_HFLAG_BDS32)) {
IMHO it would look nicer if you made this condition shorter by ORing BDS
hflags.
Feel free to add:
Reviewed-by: Leon Alrae <address@hidden>
Regards,
Leon
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2] target-mips: fix broken MIPS16 and microMIPS,
Leon Alrae <=