Re: [Qemu-devel] [PATCH v3 04/10] tcg: Introduce atomic helpers for integer min/max

[Peter Maydell]

On 8 May 2018 at 16:14, Richard Henderson <address@hidden> wrote:
> Given that this atomic operation will be used by both risc-v
> and aarch64, let's not duplicate code across the two targets.
>
> Reviewed-by: Peter Maydell <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  accel/tcg/atomic_template.h | 71 +++++++++++++++++++++++++++++++++++++
>  accel/tcg/tcg-runtime.h     |  8 +++++
>  tcg/tcg-op.h                | 34 ++++++++++++++++++
>  tcg/tcg.h                   |  8 +++++
>  tcg/tcg-op.c                |  8 +++++
>  5 files changed, 129 insertions(+)
>
> diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
> index e022df4571..2489dd3ec1 100644
> --- a/accel/tcg/atomic_template.h
> +++ b/accel/tcg/atomic_template.h
> @@ -25,18 +25,22 @@
>  #elif DATA_SIZE == 8
>  # define SUFFIX     q
>  # define DATA_TYPE  uint64_t
> +# define SDATA_TYPE int64_t
>  # define BSWAP      bswap64
>  #elif DATA_SIZE == 4
>  # define SUFFIX     l
>  # define DATA_TYPE  uint32_t
> +# define SDATA_TYPE int32_t
>  # define BSWAP      bswap32
>  #elif DATA_SIZE == 2
>  # define SUFFIX     w
>  # define DATA_TYPE  uint16_t
> +# define SDATA_TYPE int16_t
>  # define BSWAP      bswap16
>  #elif DATA_SIZE == 1
>  # define SUFFIX     b
>  # define DATA_TYPE  uint8_t
> +# define SDATA_TYPE int8_t
>  # define BSWAP
>  #else
>  # error unsupported data size
> @@ -118,6 +122,39 @@ GEN_ATOMIC_HELPER(or_fetch)
>  GEN_ATOMIC_HELPER(xor_fetch)
>
>  #undef GEN_ATOMIC_HELPER
> +
> +/* These helpers are, as a whole, full barriers.  Within the helper,
> + * the leading barrier is explicit and the trailing barrier is within
> + * cmpxchg primitive.
> + */
> +#define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET)                \
> +ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,       \
> +                        ABI_TYPE xval EXTRA_ARGS)                   \
> +{                                                                   \
> +    ATOMIC_MMU_DECLS;                                               \
> +    XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;                          \
> +    XDATA_TYPE cmp, old, new, val = xval;                           \
> +    smp_mb();                                                       \
> +    cmp = atomic_read__nocheck(haddr);                              \
> +    do {                                                            \
> +        old = cmp; new = FN(old, val);                              \
> +        cmp = atomic_cmpxchg__nocheck(haddr, old, new);             \
> +    } while (cmp != old);                                           \
> +    ATOMIC_MMU_CLEANUP;                                             \
> +    return RET;                                                     \
> +}
> +
> +GEN_ATOMIC_HELPER_FN(fetch_smin, MIN, SDATA_TYPE, old)
> +GEN_ATOMIC_HELPER_FN(fetch_umin, MIN,  DATA_TYPE, old)
> +GEN_ATOMIC_HELPER_FN(fetch_smax, MAX, SDATA_TYPE, old)
> +GEN_ATOMIC_HELPER_FN(fetch_umax, MAX,  DATA_TYPE, old)
> +
> +GEN_ATOMIC_HELPER_FN(smin_fetch, MIN, SDATA_TYPE, new)
> +GEN_ATOMIC_HELPER_FN(umin_fetch, MIN,  DATA_TYPE, new)
> +GEN_ATOMIC_HELPER_FN(smax_fetch, MAX, SDATA_TYPE, new)
> +GEN_ATOMIC_HELPER_FN(umax_fetch, MAX,  DATA_TYPE, new)

This fails to compile for me:
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1062:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_fetch_sminb_mmu’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:147:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(fetch_smin, MIN, SDATA_TYPE, old)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1062:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_fetch_smaxb_mmu’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:149:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(fetch_smax, MAX, SDATA_TYPE, old)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1062:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_smin_fetchb_mmu’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:152:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(smin_fetch, MIN, SDATA_TYPE, new)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1062:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_smax_fetchb_mmu’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:154:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(smax_fetch, MAX, SDATA_TYPE, new)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1090:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_fetch_sminb’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:147:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(fetch_smin, MIN, SDATA_TYPE, old)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1090:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_fetch_smaxb’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:149:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(fetch_smax, MAX, SDATA_TYPE, old)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1090:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_smin_fetchb’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:152:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(smin_fetch, MIN, SDATA_TYPE, new)
 ^
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1090:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:
In function ‘helper_atomic_smax_fetchb’:
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/atomic_template.h:154:1:
error: value computed is not used [-Werror=unused-value]
 GEN_ATOMIC_HELPER_FN(smax_fetch, MAX, SDATA_TYPE, new)
 ^

gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
configure arguments
exec '../../configure'
'--target-list=arm-softmmu,aarch64-softmmu,arm-linux-user,aarch64-linux-user'
'--enable-debug' '--cc=ccache gcc' '--audio-drv-list=pa'
'--with-pkgversion=pm215' "$@"

For some reason, only the signed versions get warnings...

thanks
-- PMM