[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 06/22] target/loongarch: Add main translation routines
From: |
Richard Henderson |
Subject: |
Re: [PATCH v2 06/22] target/loongarch: Add main translation routines |
Date: |
Thu, 22 Jul 2021 13:50:38 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
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
20210709042608.883256-1-richard.henderson@linaro.org/">https://patchew.org/QEMU/20210709042608.883256-1-richard.henderson@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~
- [PATCH v2 19/22] target/loongarch: Add disassembler, (continued)
- [PATCH v2 19/22] target/loongarch: Add disassembler, Song Gao, 2021/07/21
- [PATCH v2 22/22] target/loongarch: Add target build suport, Song Gao, 2021/07/21
- [PATCH v2 20/22] LoongArch Linux User Emulation, Song Gao, 2021/07/21
- [PATCH v2 21/22] configs: Add loongarch linux-user config, Song Gao, 2021/07/21
- [PATCH v2 05/22] target/loongarch: Add memory management support, Song Gao, 2021/07/21
- [PATCH v2 06/22] target/loongarch: Add main translation routines, Song Gao, 2021/07/21
- Re: [PATCH v2 06/22] target/loongarch: Add main translation routines,
Richard Henderson <=