Re: [Qemu-devel] [PATCH 08/67] target/arm: Use store_reg_from_load in thumb2 code

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
<address@hidden> wrote:
>
> There is an extra always-true ARMv5 test, but this will
> become more obvious once we start unifying the
> implementation of A32+T32.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 5e2dd8bb16..e316eeb312 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9773,13 +9773,11 @@ static void disas_thumb2_insn(DisasContext *s, 
> uint32_t insn)
>                          /* Load.  */
>                          tmp = tcg_temp_new_i32();
>                          gen_aa32_ld32u(s, tmp, addr, get_mem_index(s));
> -                        if (i == 15) {
> -                            gen_bx_excret(s, tmp);
> -                        } else if (i == rn) {
> +                        if (i == rn) {
>                              loaded_var = tmp;
>                              loaded_base = 1;
>                          } else {
> -                            store_reg(s, i, tmp);
> +                            store_reg_from_load(s, i, tmp);
>                          }
>                      } else {
>                          /* Store.  */

I thought at first this would change behaviour if rn == 15
but in practice it doesn't because 15 is the end of the loop
anyway. (This is an UNPREDICTABLE case, but if they get in our
way it might be better to explicitly make them UNDEF rather than
just behaving like whatever falls out from the implementation.)

Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM