From: Alistair Francis <alistair.francis@wdc.com>

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index e356fc6c46..25670be435 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -485,20 +485,20 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
 /* Include the auto-generated decoder for 16 bit insn */
 #include "decode-insn16.c.inc"
 
-static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
+static void decode_opc(CPURISCVState *env, DisasContext *ctx)
 {
     /* check for compressed insn */
-    if (extract16(opcode, 0, 2) != 3) {
+    if (extract16(ctx->opcode, 0, 2) != 3) {
         if (!has_ext(ctx, RVC)) {
             gen_exception_illegal(ctx);
         } else {
             ctx->pc_succ_insn = ctx->base.pc_next + 2;
-            if (!decode_insn16(ctx, opcode)) {
+            if (!decode_insn16(ctx, ctx->opcode)) {
                 gen_exception_illegal(ctx);
             }
         }
     } else {
-        uint32_t opcode32 = opcode;
+        uint32_t opcode32 = ctx->opcode;
         opcode32 = deposit32(opcode32, 16, 16,
                              translator_lduw(env, ctx->base.pc_next + 2));
         ctx->pc_succ_insn = ctx->base.pc_next + 4;
@@ -561,9 +561,9 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPURISCVState *env = cpu->env_ptr;
-    uint16_t opcode16 = translator_lduw(env, ctx->base.pc_next);
+    ctx->opcode = translator_lduw(env, ctx->base.pc_next);
 
-    decode_opc(env, ctx, opcode16);
+    decode_opc(env, ctx);
     ctx->base.pc_next = ctx->pc_succ_insn;
     ctx->w = false;
 
-- 
2.31.1

On Wed, Sep 8, 2021 at 12:54 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/translate.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

On 9/8/21 6:54 AM, Alistair Francis wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/translate.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index e356fc6c46..25670be435 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -485,20 +485,20 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>   /* Include the auto-generated decoder for 16 bit insn */
>   #include "decode-insn16.c.inc"
>   
> -static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> +static void decode_opc(CPURISCVState *env, DisasContext *ctx)
>   {
>       /* check for compressed insn */
> -    if (extract16(opcode, 0, 2) != 3) {
> +    if (extract16(ctx->opcode, 0, 2) != 3) {
>           if (!has_ext(ctx, RVC)) {
>               gen_exception_illegal(ctx);
>           } else {
>               ctx->pc_succ_insn = ctx->base.pc_next + 2;
> -            if (!decode_insn16(ctx, opcode)) {
> +            if (!decode_insn16(ctx, ctx->opcode)) {
>                   gen_exception_illegal(ctx);
>               }
>           }
>       } else {
> -        uint32_t opcode32 = opcode;
> +        uint32_t opcode32 = ctx->opcode;
>           opcode32 = deposit32(opcode32, 16, 16,
>                                translator_lduw(env, ctx->base.pc_next + 2));

You needed to write back to ctx->opcode here.

I think that all of the other changes are less than ideal -- let the value stay in a 
register as long as possible and drop them to memory immediately before calling 
decode_insn{16,32}, just before the write to pc_succ_insn in both cases.


r~


>           ctx->pc_succ_insn = ctx->base.pc_next + 4;
> @@ -561,9 +561,9 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>   {
>       DisasContext *ctx = container_of(dcbase, DisasContext, base);
>       CPURISCVState *env = cpu->env_ptr;
> -    uint16_t opcode16 = translator_lduw(env, ctx->base.pc_next);
> +    ctx->opcode = translator_lduw(env, ctx->base.pc_next);
>   
> -    decode_opc(env, ctx, opcode16);
> +    decode_opc(env, ctx);
>       ctx->base.pc_next = ctx->pc_succ_insn;
>       ctx->w = false;
>   
>