On 06/20/2018 05:05 AM, Yongbok Kim wrote: > case NM_P48I: > + insn = cpu_lduw_code(env, ctx->base.pc_next + 4);
Surely split this case out to a new function. And properly form the common, signed 32-bit offset once before the switch. > + switch ((ctx->opcode >> 16) & 0x1f) { > + case NM_LI48: > + if (rt != 0) { > + tcg_gen_movi_tl(cpu_gpr[rt], > + extract32(ctx->opcode, 0, 16) | insn << 16); The 32-bit constant is to be sign-extended for nanomips 64. > + } > + break; > + case NM_ADDIU48: > + if (rt != 0) { > + tcg_gen_addi_tl(cpu_gpr[rt], cpu_gpr[rt], > + extract32(ctx->opcode, 0, 16) | insn << 16); Likewise. > + tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]); > + } > + break; > + case NM_ADDIUGP48: > + if (rt != 0) { > + tcg_gen_addi_tl(cpu_gpr[rt], cpu_gpr[28], > + extract32(ctx->opcode, 0, 16) | insn << 16); Likewise. > + tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]); Behaves-like DADDIU[GP48]. Which would indicate using gen_op_addr_add[i]. I know you're only targeting nanomips32 now, but I think you need to be clearer about all of these 64-bit edge cases now, lest they be very difficult to pick out later. > + } > + break; > + case NM_ADDIUPC48: > + if (rt != 0) { > + int32_t offset = extract32(ctx->opcode, 0, 16) | insn << 16; > + target_long addr = addr_add(ctx, ctx->base.pc_next + 6, > offset); > + > + tcg_gen_movi_tl(cpu_gpr[rt], addr); > + tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]); Likewise. And the ext32s would be doubly redundant with that already done in addr_add. r~