Re: [Qemu-devel] [PATCH v3 4/9] target/arm: Check HAVE_CMPXCHG128 at translate time

[Philippe Mathieu-Daudé]

On 03/10/2018 21:39, Richard Henderson wrote:
> Reviewed-by: Emilio G. Cota <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  target/arm/helper-a64.c    | 16 ++++------------
>  target/arm/translate-a64.c | 38 ++++++++++++++++++++++----------------
>  2 files changed, 26 insertions(+), 28 deletions(-)
> 
> diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
> index 6e4e1b8a19..61799d20e1 100644
> --- a/target/arm/helper-a64.c
> +++ b/target/arm/helper-a64.c
> @@ -563,9 +563,7 @@ uint64_t HELPER(paired_cmpxchg64_le_parallel)(CPUARMState 
> *env, uint64_t addr,
>      int mem_idx;
>      TCGMemOpIdx oi;
>  
> -    if (!HAVE_CMPXCHG128) {
> -        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> -    }
> +    assert(HAVE_CMPXCHG128);
>  
>      mem_idx = cpu_mmu_index(env, false);
>      oi = make_memop_idx(MO_LEQ | MO_ALIGN_16, mem_idx);
> @@ -635,9 +633,7 @@ uint64_t HELPER(paired_cmpxchg64_be_parallel)(CPUARMState 
> *env, uint64_t addr,
>      int mem_idx;
>      TCGMemOpIdx oi;
>  
> -    if (!HAVE_CMPXCHG128) {
> -        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> -    }
> +    assert(HAVE_CMPXCHG128);
>  
>      mem_idx = cpu_mmu_index(env, false);
>      oi = make_memop_idx(MO_BEQ | MO_ALIGN_16, mem_idx);
> @@ -663,9 +659,7 @@ void HELPER(casp_le_parallel)(CPUARMState *env, uint32_t 
> rs, uint64_t addr,
>      int mem_idx;
>      TCGMemOpIdx oi;
>  
> -    if (!HAVE_CMPXCHG128) {
> -        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> -    }
> +    assert(HAVE_CMPXCHG128);
>  
>      mem_idx = cpu_mmu_index(env, false);
>      oi = make_memop_idx(MO_LEQ | MO_ALIGN_16, mem_idx);
> @@ -686,9 +680,7 @@ void HELPER(casp_be_parallel)(CPUARMState *env, uint32_t 
> rs, uint64_t addr,
>      int mem_idx;
>      TCGMemOpIdx oi;
>  
> -    if (!HAVE_CMPXCHG128) {
> -        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> -    }
> +    assert(HAVE_CMPXCHG128);
>  
>      mem_idx = cpu_mmu_index(env, false);
>      oi = make_memop_idx(MO_LEQ | MO_ALIGN_16, mem_idx);
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 8ca3876707..77ee8d9085 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -37,6 +37,7 @@
>  
>  #include "trace-tcg.h"
>  #include "translate-a64.h"
> +#include "qemu/atomic128.h"
>  
>  static TCGv_i64 cpu_X[32];
>  static TCGv_i64 cpu_pc;
> @@ -2082,26 +2083,27 @@ static void gen_store_exclusive(DisasContext *s, int 
> rd, int rt, int rt2,
>                                         get_mem_index(s),
>                                         MO_64 | MO_ALIGN | s->be_data);
>              tcg_gen_setcond_i64(TCG_COND_NE, tmp, tmp, cpu_exclusive_val);
> -        } else if (s->be_data == MO_LE) {
> -            if (tb_cflags(s->base.tb) & CF_PARALLEL) {
> +        } else if (tb_cflags(s->base.tb) & CF_PARALLEL) {
> +            if (!HAVE_CMPXCHG128) {
> +                gen_helper_exit_atomic(cpu_env);
> +                s->base.is_jmp = DISAS_NORETURN;
> +            } else if (s->be_data == MO_LE) {
>                  gen_helper_paired_cmpxchg64_le_parallel(tmp, cpu_env,
>                                                          cpu_exclusive_addr,
>                                                          cpu_reg(s, rt),
>                                                          cpu_reg(s, rt2));
>              } else {
> -                gen_helper_paired_cmpxchg64_le(tmp, cpu_env, 
> cpu_exclusive_addr,
> -                                               cpu_reg(s, rt), cpu_reg(s, 
> rt2));
> -            }
> -        } else {
> -            if (tb_cflags(s->base.tb) & CF_PARALLEL) {
>                  gen_helper_paired_cmpxchg64_be_parallel(tmp, cpu_env,
>                                                          cpu_exclusive_addr,
>                                                          cpu_reg(s, rt),
>                                                          cpu_reg(s, rt2));
> -            } else {
> -                gen_helper_paired_cmpxchg64_be(tmp, cpu_env, 
> cpu_exclusive_addr,
> -                                               cpu_reg(s, rt), cpu_reg(s, 
> rt2));
>              }
> +        } else if (s->be_data == MO_LE) {
> +            gen_helper_paired_cmpxchg64_le(tmp, cpu_env, cpu_exclusive_addr,
> +                                           cpu_reg(s, rt), cpu_reg(s, rt2));
> +        } else {
> +            gen_helper_paired_cmpxchg64_be(tmp, cpu_env, cpu_exclusive_addr,
> +                                           cpu_reg(s, rt), cpu_reg(s, rt2));
>          }
>      } else {
>          tcg_gen_atomic_cmpxchg_i64(tmp, cpu_exclusive_addr, 
> cpu_exclusive_val,
> @@ -2171,14 +2173,18 @@ static void gen_compare_and_swap_pair(DisasContext 
> *s, int rs, int rt,
>          }
>          tcg_temp_free_i64(cmp);
>      } else if (tb_cflags(s->base.tb) & CF_PARALLEL) {
> -        TCGv_i32 tcg_rs = tcg_const_i32(rs);
> -
> -        if (s->be_data == MO_LE) {
> -            gen_helper_casp_le_parallel(cpu_env, tcg_rs, addr, t1, t2);
> +        if (HAVE_CMPXCHG128) {
> +            TCGv_i32 tcg_rs = tcg_const_i32(rs);
> +            if (s->be_data == MO_LE) {
> +                gen_helper_casp_le_parallel(cpu_env, tcg_rs, addr, t1, t2);
> +            } else {
> +                gen_helper_casp_be_parallel(cpu_env, tcg_rs, addr, t1, t2);
> +            }
> +            tcg_temp_free_i32(tcg_rs);
>          } else {
> -            gen_helper_casp_be_parallel(cpu_env, tcg_rs, addr, t1, t2);
> +            gen_helper_exit_atomic(cpu_env);
> +            s->base.is_jmp = DISAS_NORETURN;
>          }
> -        tcg_temp_free_i32(tcg_rs);
>      } else {
>          TCGv_i64 d1 = tcg_temp_new_i64();
>          TCGv_i64 d2 = tcg_temp_new_i64();
>