Re: [Qemu-devel] [RFC v5 4/6] target-arm: Create new runtime helpers for excl accesses

[Richard Henderson <rth@twiddle.net>]

On 09/24/2015 06:32 PM, Alvise Rigo wrote:
> Introduce a set of new runtime helpers do handle exclusive instructions.
> This helpers are used as hooks to call the respective LL/SC helpers in
> softmmu_llsc_template.h from TCG code.
>
> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
>   target-arm/helper.h    | 10 ++++++
>   target-arm/op_helper.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 104 insertions(+)
>
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index 827b33d..8e7a7c2 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -530,6 +530,16 @@ DEF_HELPER_2(dc_zva, void, env, i64)
>   DEF_HELPER_FLAGS_2(neon_pmull_64_lo, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>   DEF_HELPER_FLAGS_2(neon_pmull_64_hi, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>
> +DEF_HELPER_3(ldlink_aa32_i8, i32, env, i32, i32)
> +DEF_HELPER_3(ldlink_aa32_i16, i32, env, i32, i32)
> +DEF_HELPER_3(ldlink_aa32_i32, i32, env, i32, i32)
> +DEF_HELPER_3(ldlink_aa32_i64, i64, env, i32, i32)
> +
> +DEF_HELPER_4(stcond_aa32_i8, i32, env, i32, i32, i32)
> +DEF_HELPER_4(stcond_aa32_i16, i32, env, i32, i32, i32)
> +DEF_HELPER_4(stcond_aa32_i32, i32, env, i32, i32, i32)
> +DEF_HELPER_4(stcond_aa32_i64, i32, env, i32, i64, i32)
> +
>   #ifdef TARGET_AARCH64
>   #include "helper-a64.h"
>   #endif
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 663c05d..d832ba8 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -969,3 +969,97 @@ uint32_t HELPER(ror_cc)(CPUARMState *env, uint32_t x, uint32_t i)
>           return ((uint32_t)x >> shift) | (x << (32 - shift));
>       }
>   }
> +
> +/* LoadLink helpers, only unsigned. */
> +static void * const qemu_ldex_helpers[16] = {
> +    [MO_UB]   = helper_ret_ldlinkub_mmu,
> +
> +    [MO_LEUW] = helper_le_ldlinkuw_mmu,
> +    [MO_LEUL] = helper_le_ldlinkul_mmu,
> +    [MO_LEQ]  = helper_le_ldlinkq_mmu,
> +
> +    [MO_BEUW] = helper_be_ldlinkuw_mmu,
> +    [MO_BEUL] = helper_be_ldlinkul_mmu,
> +    [MO_BEQ]  = helper_be_ldlinkq_mmu,
> +};
> +
> +#define LDEX_HELPER(SUFF, OPC)                                          \
> +uint32_t HELPER(ldlink_aa32_i##SUFF)(CPUARMState *env, uint32_t addr,   \
> +                                                       uint32_t index)  \
> +{                                                                       \
> +    CPUArchState *state = env;                                          \
> +    TCGMemOpIdx op;                                                     \
> +                                                                        \
> +    op = make_memop_idx(OPC, index);                                    \
> +                                                                        \
> +    tcg_target_ulong (*func)(CPUArchState *env, target_ulong addr,      \
> +                             TCGMemOpIdx oi, uintptr_t retaddr);        \
> +    func = qemu_ldex_helpers[OPC];                                      \
> +                                                                        \
> +    return (uint32_t)func(state, addr, op, GETRA());                    \
> +}
> +
> +LDEX_HELPER(8, MO_UB)
> +LDEX_HELPER(16, MO_TEUW)
> +LDEX_HELPER(32, MO_TEUL)

This is not what Aurelien meant.  I cannot see any reason at present why 
generic wrappers, available for all targets, shouldn't be sufficient.

See tcg/tcg-runtime.h and tcg-runtime.c.

You shouldn't need to look up a function in a table like this.  The decision 
about whether to call a BE or LE helper should have been made in the translator.


r~