On 7/20/21 11:53 PM, Song Gao wrote:
+/* General purpose registers moves. */ +void gen_load_gpr(TCGv t, int reg) +{ + if (reg == 0) { + tcg_gen_movi_tl(t, 0); + } else { + tcg_gen_mov_tl(t, cpu_gpr[reg]); + } +}
Please have a look at https://patchew.org/QEMU/20210709042608.883256-1-richard.hender...@linaro.org/ for a better way to handle the zero register.
+static inline void save_cpu_state(DisasContext *ctx, int do_save_pc) +{ + if (do_save_pc && ctx->base.pc_next != ctx->saved_pc) { + gen_save_pc(ctx->base.pc_next); + ctx->saved_pc = ctx->base.pc_next; + } + if (ctx->hflags != ctx->saved_hflags) { + tcg_gen_movi_i32(hflags, ctx->hflags); + ctx->saved_hflags = ctx->hflags; + switch (ctx->hflags & LOONGARCH_HFLAG_BMASK) { + case LOONGARCH_HFLAG_BR: + break; + case LOONGARCH_HFLAG_BC: + case LOONGARCH_HFLAG_B: + tcg_gen_movi_tl(btarget, ctx->btarget); + break; + } + } +}
Drop all the hflags handling. It's all copied from mips delay slot handling.
+ +static inline void restore_cpu_state(CPULoongArchState *env, DisasContext *ctx) +{ + ctx->saved_hflags = ctx->hflags; + switch (ctx->hflags & LOONGARCH_HFLAG_BMASK) { + case LOONGARCH_HFLAG_BR: + break; + case LOONGARCH_HFLAG_BC: + case LOONGARCH_HFLAG_B: + ctx->btarget = env->btarget; + break; + } +}
Likewise.
+static void gen_load_fpr32h(TCGv_i32 t, int reg) +{ + tcg_gen_extrh_i64_i32(t, fpu_f64[reg]); +} + +static void gen_store_fpr32h(TCGv_i32 t, int reg) +{ + TCGv_i64 t64 = tcg_temp_new_i64(); + tcg_gen_extu_i32_i64(t64, t); + tcg_gen_deposit_i64(fpu_f64[reg], fpu_f64[reg], t64, 32, 32); + tcg_temp_free_i64(t64); +}
There is no general-purpose high-part fpr stuff. There's only movgr2frh and movfrh2gr, and you can simplify both if you drop the transition through TCGv_i32.
+void gen_op_addr_add(TCGv ret, TCGv arg0, TCGv arg1) +{ + tcg_gen_add_tl(ret, arg0, arg1); +}
No point in this, since loongarch has no 32-bit address mode.
+void gen_base_offset_addr(TCGv addr, int base, int offset) +{ + if (base == 0) { + tcg_gen_movi_tl(addr, offset); + } else if (offset == 0) { + gen_load_gpr(addr, base); + } else { + tcg_gen_movi_tl(addr, offset); + gen_op_addr_add(addr, cpu_gpr[base], addr); + } +}
Using the interfaces I quote above from my riscv cleanup, this can be tidied to tcg_gen_addi_tl(addr, gpr_src(base), offset);
+static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest) +{ + return true; +}
You must now use translate_use_goto_tb, which will not always return true. You will see assertion failures otherwise.
+static inline void clear_branch_hflags(DisasContext *ctx) +{ + ctx->hflags &= ~LOONGARCH_HFLAG_BMASK; + if (ctx->base.is_jmp == DISAS_NEXT) { + save_cpu_state(ctx, 0); + } else { + /* + * It is not safe to save ctx->hflags as hflags may be changed + * in execution time. + */ + tcg_gen_andi_i32(hflags, hflags, ~LOONGARCH_HFLAG_BMASK); + } +}
Not required.
+static void gen_branch(DisasContext *ctx, int insn_bytes) +{ + if (ctx->hflags & LOONGARCH_HFLAG_BMASK) { + int proc_hflags = ctx->hflags & LOONGARCH_HFLAG_BMASK; + /* Branches completion */ + clear_branch_hflags(ctx); + ctx->base.is_jmp = DISAS_NORETURN; + switch (proc_hflags & LOONGARCH_HFLAG_BMASK) { + case LOONGARCH_HFLAG_B: + /* unconditional branch */ + gen_goto_tb(ctx, 0, ctx->btarget); + break; + case LOONGARCH_HFLAG_BC: + /* Conditional branch */ + { + TCGLabel *l1 = gen_new_label(); + + tcg_gen_brcondi_tl(TCG_COND_NE, bcond, 0, l1); + gen_goto_tb(ctx, 1, ctx->base.pc_next + insn_bytes); + gen_set_label(l1); + gen_goto_tb(ctx, 0, ctx->btarget); + } + break; + case LOONGARCH_HFLAG_BR: + /* unconditional branch to register */ + tcg_gen_mov_tl(cpu_PC, btarget); + tcg_gen_lookup_and_goto_ptr(); + break; + default: + fprintf(stderr, "unknown branch 0x%x\n", proc_hflags); + abort(); + } + } +}
Split this up into the various trans_* branch routines, without the setting of HFLAG.
+static void loongarch_tr_init_disas_context(DisasContextBase *dcbase, + CPUState *cs) +{ + DisasContext *ctx = container_of(dcbase, DisasContext, base); + CPULoongArchState *env = cs->env_ptr; + + ctx->page_start = ctx->base.pc_first & TARGET_PAGE_MASK; + ctx->saved_pc = -1; + ctx->btarget = 0; + /* Restore state from the tb context. */ + ctx->hflags = (uint32_t)ctx->base.tb->flags; + restore_cpu_state(env, ctx); + ctx->mem_idx = LOONGARCH_HFLAG_UM;
This is not an mmu index. You didn't notice the error because you're only doing user-mode. You're missing a check for page crossing. Generally, for fixed-width ISAs like this, we do /* Bound the number of insns to execute to those left on the page. */ int bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4; ctx->base.max_insns = MIN(ctx->base.max_insns, bound); here in init_disas_context.
+static void loongarch_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ + DisasContext *ctx = container_of(dcbase, DisasContext, base); + + tcg_gen_insn_start(ctx->base.pc_next, ctx->hflags & LOONGARCH_HFLAG_BMASK, + ctx->btarget);
No hflags/btarget stuff. Drop TARGET_INSN_START_EXTRA_WORDS.
+static bool loongarch_tr_breakpoint_check(DisasContextBase *dcbase, + CPUState *cs, + const CPUBreakpoint *bp) +{ + return true; +}
Broken, but now handled generically, so remove it.
+static void loongarch_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) +{ + CPULoongArchState *env = cs->env_ptr; + DisasContext *ctx = container_of(dcbase, DisasContext, base); + int insn_bytes = 4; + + ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next); + + if (!decode(ctx, ctx->opcode)) { + fprintf(stderr, "Error: unkown opcode. 0x%lx: 0x%x\n", + ctx->base.pc_next, ctx->opcode);
No fprintfs. Use qemu_log_mask with LOG_UNIMP or LOG_GUEST_ERROR.
+ if (ctx->hflags & LOONGARCH_HFLAG_BMASK) { + gen_branch(ctx, insn_bytes); + }
Drop this, as I mentioned above.
+static void fpu_dump_state(CPULoongArchState *env, FILE * f, int flags) +{ + int i; + int is_fpu64 = 1; + +#define printfpr(fp) \ + do { \ + if (is_fpu64) \ + qemu_fprintf(f, "w:%08x d:%016" PRIx64 \ + " fd:%13g fs:%13g psu: %13g\n", \ + (fp)->w[FP_ENDIAN_IDX], (fp)->d, \ + (double)(fp)->fd, \ + (double)(fp)->fs[FP_ENDIAN_IDX], \ + (double)(fp)->fs[!FP_ENDIAN_IDX]); \ + else { \ + fpr_t tmp; \ + tmp.w[FP_ENDIAN_IDX] = (fp)->w[FP_ENDIAN_IDX]; \ + tmp.w[!FP_ENDIAN_IDX] = ((fp) + 1)->w[FP_ENDIAN_IDX]; \ + qemu_fprintf(f, "w:%08x d:%016" PRIx64 \ + " fd:%13g fs:%13g psu:%13g\n", \ + tmp.w[FP_ENDIAN_IDX], tmp.d, \ + (double)tmp.fd, \ + (double)tmp.fs[FP_ENDIAN_IDX], \ + (double)tmp.fs[!FP_ENDIAN_IDX]); \ + } \ + } while (0)
This is broken. You're performing an integer to fp conversion of something that is already a floating-point value, not printing the floating-point value itself. It's broken in the mips code as well.
In addition, is_fpu64 is pointless for loongarch.
+void loongarch_tcg_init(void) +{ + int i; + + for (i = 0; i < 32; i++) + cpu_gpr[i] = tcg_global_mem_new(cpu_env, + offsetof(CPULoongArchState, + active_tc.gpr[i]), + regnames[i]);
Missing braces. Do not create a temp for the zero register.
+ bcond = tcg_global_mem_new(cpu_env, + offsetof(CPULoongArchState, bcond), "bcond"); + btarget = tcg_global_mem_new(cpu_env, + offsetof(CPULoongArchState, btarget), + "btarget"); + hflags = tcg_global_mem_new_i32(cpu_env, + offsetof(CPULoongArchState, hflags), + "hflags");
Drop these. r~