| Message ID | 1294412794-25573-2-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 07, 2011 at 03:06:28PM +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. Have you considered using tb flags instead? On the other hand I am not sure it will make a real difference. > 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(-) > > 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 > > >
On 7 January 2011 16:01, Aurelien Jarno <aurelien@aurel32.net> wrote: > On Fri, Jan 07, 2011 at 03:06:28PM +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. > > Have you considered using tb flags instead? On the other hand I am not > sure it will make a real difference. I thought about that, but: (a) if we put the current mode into the tb->flags then TBs which could previously have been shared between several modes now have to be translated as separate TBs for each mode (b) it would eat 5 bits of tb->flags and we only have 16 left (c) SRS isn't a very commonly used instruction anyway (and the overhead of taking the exception will dwarf the call out to the helper for SRS) -- PMM
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); }
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(-)