diff mbox

[5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn()

Message ID 1414524244-20316-6-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell Oct. 28, 2014, 7:24 p.m. UTC
Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
we move the "read insn from memory" code to the callsite and pass the
insn to the function instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Alex Bennée Oct. 31, 2014, 1:47 p.m. UTC | #1
Peter Maydell <peter.maydell@linaro.org> writes:

> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
> we move the "read insn from memory" code to the callsite and pass the
> insn to the function instead.
>
<snip>
>  
> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> +static void disas_arm_insn(DisasContext *s, unsigned int insn)

I note that in the aarch64 code we used the unambiguous uint32_t for the
insn type. I'm hard pressed to imagine it actually breaking anything
though.



>  {
> -    unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
> +    unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
>      TCGv_i32 tmp;
>      TCGv_i32 tmp2;
>      TCGv_i32 tmp3;
>      TCGv_i32 addr;
>      TCGv_i64 tmp64;
>  
> -    insn = arm_ldl_code(env, s->pc, s->bswap_code);
> -    s->pc += 4;
> -
>      /* M variants do not implement ARM mode.  */
>      if (arm_dc_feature(s, ARM_FEATURE_M)) {
>          goto illegal_op;
> @@ -11199,7 +11196,9 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
>                  }
>              }
>          } else {
> -            disas_arm_insn(env, dc);
> +            unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
> +            dc->pc += 4;
> +            disas_arm_insn(dc, insn);
>          }
>  
>          if (dc->condjmp && !dc->is_jmp) {

Anyway looks fine:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Peter Maydell Nov. 4, 2014, 12:40 a.m. UTC | #2
On 31 October 2014 13:47, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
>> we move the "read insn from memory" code to the callsite and pass the
>> insn to the function instead.
>>
> <snip>
>>
>> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
>> +static void disas_arm_insn(DisasContext *s, unsigned int insn)
>
> I note that in the aarch64 code we used the unambiguous uint32_t for the
> insn type. I'm hard pressed to imagine it actually breaking anything
> though.

Yeah, we already strongly assume int to be 32 bits, and I wanted
to make this patch not mess with the type of the existing variable.

thanks
-- PMM
Claudio Fontana Nov. 4, 2014, 8:55 a.m. UTC | #3
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>

On 28.10.2014 20:24, Peter Maydell wrote:
> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
> we move the "read insn from memory" code to the callsite and pass the
> insn to the function instead.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 9e2dda2..932fa03 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7560,18 +7560,15 @@ static void gen_srs(DisasContext *s,
>      tcg_temp_free_i32(addr);
>  }
>  
> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> +static void disas_arm_insn(DisasContext *s, unsigned int insn)
>  {
> -    unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
> +    unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
>      TCGv_i32 tmp;
>      TCGv_i32 tmp2;
>      TCGv_i32 tmp3;
>      TCGv_i32 addr;
>      TCGv_i64 tmp64;
>  
> -    insn = arm_ldl_code(env, s->pc, s->bswap_code);
> -    s->pc += 4;
> -
>      /* M variants do not implement ARM mode.  */
>      if (arm_dc_feature(s, ARM_FEATURE_M)) {
>          goto illegal_op;
> @@ -11199,7 +11196,9 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
>                  }
>              }
>          } else {
> -            disas_arm_insn(env, dc);
> +            unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
> +            dc->pc += 4;
> +            disas_arm_insn(dc, insn);
>          }
>  
>          if (dc->condjmp && !dc->is_jmp) {
>
diff mbox

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9e2dda2..932fa03 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7560,18 +7560,15 @@  static void gen_srs(DisasContext *s,
     tcg_temp_free_i32(addr);
 }
 
-static void disas_arm_insn(CPUARMState * env, DisasContext *s)
+static void disas_arm_insn(DisasContext *s, unsigned int insn)
 {
-    unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
+    unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
     TCGv_i32 tmp;
     TCGv_i32 tmp2;
     TCGv_i32 tmp3;
     TCGv_i32 addr;
     TCGv_i64 tmp64;
 
-    insn = arm_ldl_code(env, s->pc, s->bswap_code);
-    s->pc += 4;
-
     /* M variants do not implement ARM mode.  */
     if (arm_dc_feature(s, ARM_FEATURE_M)) {
         goto illegal_op;
@@ -11199,7 +11196,9 @@  static inline void gen_intermediate_code_internal(ARMCPU *cpu,
                 }
             }
         } else {
-            disas_arm_insn(env, dc);
+            unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
+            dc->pc += 4;
+            disas_arm_insn(dc, insn);
         }
 
         if (dc->condjmp && !dc->is_jmp) {