qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 01/29] target/riscv: Move CPURISCVState point


From: Palmer Dabbelt
Subject: Re: [Qemu-devel] [PATCH v2 01/29] target/riscv: Move CPURISCVState pointer to DisasContext
Date: Thu, 25 Oct 2018 09:38:27 -0700 (PDT)

On Sat, 20 Oct 2018 00:14:23 PDT (-0700), address@hidden wrote:
CPURISCVState is rarely used, so there is no need to pass it to every
translate function. This paves the way for decodetree which only passes
DisasContext to translate functions.

Signed-off-by: Bastian Koppelmann <address@hidden>
---
 target/riscv/translate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 18d7b6d147..e81b9f097e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -52,6 +52,7 @@ typedef struct DisasContext {
        to any system register, which includes CSR_FRM, so we do not have
        to reset this known value.  */
     int frm;
+    CPURISCVState *env;
 } DisasContext;

 /* convert riscv funct3 to qemu memop for load/store */
@@ -1789,19 +1790,19 @@ static void decode_RV32_64G(CPURISCVState *env, 
DisasContext *ctx)
     }
 }

-static void decode_opc(CPURISCVState *env, DisasContext *ctx)
+static void decode_opc(DisasContext *ctx)
 {
     /* check for compressed insn */
     if (extract32(ctx->opcode, 0, 2) != 3) {
-        if (!riscv_has_ext(env, RVC)) {
+        if (!riscv_has_ext(ctx->env, RVC)) {
             gen_exception_illegal(ctx);
         } else {
             ctx->pc_succ_insn = ctx->base.pc_next + 2;
-            decode_RV32_64C(env, ctx);
+            decode_RV32_64C(ctx->env, ctx);
         }
     } else {
         ctx->pc_succ_insn = ctx->base.pc_next + 4;
-        decode_RV32_64G(env, ctx);
+        decode_RV32_64G(ctx->env, ctx);
     }
 }

@@ -1846,10 +1847,10 @@ static bool riscv_tr_breakpoint_check(DisasContextBase 
*dcbase, CPUState *cpu,
 static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
-    CPURISCVState *env = cpu->env_ptr;
+    ctx->env = cpu->env_ptr;

-    ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
-    decode_opc(env, ctx);
+    ctx->opcode = cpu_ldl_code(ctx->env, ctx->base.pc_next);
+    decode_opc(ctx);
     ctx->base.pc_next = ctx->pc_succ_insn;

     if (ctx->base.is_jmp == DISAS_NEXT) {

I think this is OK, but I'm not sure. All the other ports do this in a different way: rather than including the CPU state structure in DisasContext they explicitly call out the state that can change instruction decoding by duplicating it within DisasContext. Michael's patch queue handles this in the canonical way, but I think it's a bit cleaner to avoid duplicating the state. My worry is that, since changing the CPU state changes the legal instructions, we will paint ourselves into a corner when it comes to faithfully implementing a writeable MISA register.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]