Re: [Qemu-devel] [PATCH 1/8] target-arm: Don't generate code specific to current CPU mode for SRS

[Aurelien Jarno <aurelien@aurel32.net>]

On Tue, Jan 11, 2011 at 10:12:11PM +0000, Peter Maydell wrote:
> When translating the SRS instruction, handle the "store registers
> to stack of current mode" case in the helper function rather than
> inline. This means the generated code does not make assumptions
> about the current CPU mode which might not be valid when the TB
> is executed later.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c    |   12 ++++++++++--
>  target-arm/translate.c |   46 +++++++++++++++-------------------------------
>  2 files changed, 25 insertions(+), 33 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 705b99f..f08e09e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1846,12 +1846,20 @@ bad_reg:
>  
>  void HELPER(set_r13_banked)(CPUState *env, uint32_t mode, uint32_t val)
>  {
> -    env->banked_r13[bank_number(mode)] = val;
> +    if ((env->uncached_cpsr & CPSR_M) == mode) {
> +        env->regs[13] = val;
> +    } else {
> +        env->banked_r13[bank_number(mode)] = val;
> +    }
>  }
>  
>  uint32_t HELPER(get_r13_banked)(CPUState *env, uint32_t mode)
>  {
> -    return env->banked_r13[bank_number(mode)];
> +    if ((env->uncached_cpsr & CPSR_M) == mode) {
> +        return env->regs[13];
> +    } else {
> +        return env->banked_r13[bank_number(mode)];
> +    }
>  }
>  
>  uint32_t HELPER(v7m_mrs)(CPUState *env, uint32_t reg)
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 2ce82f3..c391398 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6101,14 +6101,10 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                  goto illegal_op;
>              ARCH(6);
>              op1 = (insn & 0x1f);
> -            if (op1 == (env->uncached_cpsr & CPSR_M)) {
> -                addr = load_reg(s, 13);
> -            } else {
> -                addr = new_tmp();
> -                tmp = tcg_const_i32(op1);
> -                gen_helper_get_r13_banked(addr, cpu_env, tmp);
> -                tcg_temp_free_i32(tmp);
> -            }
> +            addr = new_tmp();
> +            tmp = tcg_const_i32(op1);
> +            gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +            tcg_temp_free_i32(tmp);
>              i = (insn >> 23) & 3;
>              switch (i) {
>              case 0: offset = -4; break; /* DA */
> @@ -6135,14 +6131,10 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                  }
>                  if (offset)
>                      tcg_gen_addi_i32(addr, addr, offset);
> -                if (op1 == (env->uncached_cpsr & CPSR_M)) {
> -                    store_reg(s, 13, addr);
> -                } else {
> -                    tmp = tcg_const_i32(op1);
> -                    gen_helper_set_r13_banked(cpu_env, tmp, addr);
> -                    tcg_temp_free_i32(tmp);
> -                    dead_tmp(addr);
> -                }
> +                tmp = tcg_const_i32(op1);
> +                gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                tcg_temp_free_i32(tmp);
> +                dead_tmp(addr);
>              } else {
>                  dead_tmp(addr);
>              }
> @@ -7554,14 +7546,10 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                  } else {
>                      /* srs */
>                      op = (insn & 0x1f);
> -                    if (op == (env->uncached_cpsr & CPSR_M)) {
> -                        addr = load_reg(s, 13);
> -                    } else {
> -                        addr = new_tmp();
> -                        tmp = tcg_const_i32(op);
> -                        gen_helper_get_r13_banked(addr, cpu_env, tmp);
> -                        tcg_temp_free_i32(tmp);
> -                    }
> +                    addr = new_tmp();
> +                    tmp = tcg_const_i32(op);
> +                    gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +                    tcg_temp_free_i32(tmp);
>                      if ((insn & (1 << 24)) == 0) {
>                          tcg_gen_addi_i32(addr, addr, -8);
>                      }
> @@ -7577,13 +7565,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                          } else {
>                              tcg_gen_addi_i32(addr, addr, 4);
>                          }
> -                        if (op == (env->uncached_cpsr & CPSR_M)) {
> -                            store_reg(s, 13, addr);
> -                        } else {
> -                            tmp = tcg_const_i32(op);
> -                            gen_helper_set_r13_banked(cpu_env, tmp, addr);
> -                            tcg_temp_free_i32(tmp);
> -                        }
> +                        tmp = tcg_const_i32(op);
> +                        gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                        tcg_temp_free_i32(tmp);
>                      } else {
>                          dead_tmp(addr);
>                      }
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net