On Tue, Oct 11, 2016 at 14:40:39 -0500, Richard Henderson wrote:
> Add all of cmpxchg, op_fetch, fetch_op, and xchg.
> Handle both endian-ness, and sizes up to 8.
> Handle expanding non-atomically, when emulating in serial.
>
> Signed-off-by: Richard Henderson <r...@twiddle.net>
> ---
> Makefile.objs | 2 +-
> Makefile.target | 1 +
> atomic_template.h | 173 ++++++++++++++++++++++++++
> cputlb.c | 112 ++++++++++++++++-
> include/qemu/atomic.h | 43 ++++---
I'd have the changes to atomic.h as a separate patch--this one is
pretty big already.
> tcg-runtime.c | 49 ++++++--
> tcg/tcg-op.c | 328
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> tcg/tcg-op.h | 44 +++++++
> tcg/tcg-runtime.h | 75 ++++++++++++
> tcg/tcg.h | 53 ++++++++
> 10 files changed, 848 insertions(+), 32 deletions(-)
> create mode 100644 atomic_template.h
(snip)
> diff --git a/atomic_template.h b/atomic_template.h
> new file mode 100644
> index 0000000..d2c8a08
> --- /dev/null
> +++ b/atomic_template.h
(snip)
> +#if DATA_SIZE == 1
> +# define END
> +#elif defined(HOST_WORDS_BIGENDIAN)
> +# define END _be
> +#else
> +# define END _le
> +#endif
It took me a while to figure out that ATOMIC_NAME needs END (ATOMIC_NAME
is defined later in the patch).
It might be clearer to pass it explicitly as a suffix in the macro, as in
#define ATOMIC_NAME(name, suffix) to then have ATOMIC_NAME(cmpxchg, END).
(snip)
> +
> +/* Note that for addition, we need to use a separate cmpxchg loop instead
> + of bswaps for the reverse-host-endian helpers. */
> +ABI_TYPE ATOMIC_NAME(fetch_add)(CPUArchState *env, target_ulong addr,
> + ABI_TYPE val EXTRA_ARGS)
> +{
> + DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> + DATA_TYPE ldo, ldn, ret, sto;
> +
> + ldo = *haddr;
ldo = atomic_read(haddr)
would be better here for C11 compliance (or tsan will complain).
> + while (1) {
> + ret = BSWAP(ldo);
> + sto = BSWAP(ret + val);
> + ldn = atomic_cmpxchg__nocheck(haddr, ldo, sto);
> + if (ldn == ldo) {
> + return ret;
> + }
> + ldo = ldn;
> + }
> +}
> +
> +ABI_TYPE ATOMIC_NAME(add_fetch)(CPUArchState *env, target_ulong addr,
> + ABI_TYPE val EXTRA_ARGS)
> +{
> + DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> + DATA_TYPE ldo, ldn, ret, sto;
> +
> + ldo = *haddr;
ditto.
No more comments. Have to say though that I really like how with the
tables in tcg-op.c, code ends up looking neat!
Emilio
