[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/5] target/s390x: Convert to HAVE_CMPXCHG128 an
From: |
Emilio G. Cota |
Subject: |
Re: [Qemu-devel] [PATCH 3/5] target/s390x: Convert to HAVE_CMPXCHG128 and HAVE_ATOMIC128 |
Date: |
Fri, 17 Aug 2018 13:03:59 -0400 |
User-agent: |
Mutt/1.9.4 (2018-02-28) |
On Wed, Aug 15, 2018 at 19:54:50 -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> target/s390x/mem_helper.c | 87 +++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 50 deletions(-)
>
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index e21a47fb4d..908b1254c8 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -25,6 +25,7 @@
> #include "exec/exec-all.h"
> #include "exec/cpu_ldst.h"
> #include "qemu/int128.h"
> +#include "qemu/atomic128.h"
>
> #if !defined(CONFIG_USER_ONLY)
> #include "hw/s390x/storage-keys.h"
> @@ -1375,7 +1376,7 @@ static void do_cdsg(CPUS390XState *env, uint64_t addr,
> bool fail;
>
> if (parallel) {
> -#ifndef CONFIG_ATOMIC128
> +#if !HAVE_CMPXCHG128
> cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> #else
> int mem_idx = cpu_mmu_index(env, false);
> @@ -1421,12 +1422,10 @@ void HELPER(cdsg_parallel)(CPUS390XState *env,
> uint64_t addr,
> static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
> uint64_t a2, bool parallel)
> {
> -#if !defined(CONFIG_USER_ONLY) || defined(CONFIG_ATOMIC128)
> uint32_t mem_idx = cpu_mmu_index(env, false);
> -#endif
> uintptr_t ra = GETPC();
> - uint32_t fc = extract32(env->regs[0], 0, 8);
> - uint32_t sc = extract32(env->regs[0], 8, 8);
> + uint32_t fc = extract32(env->regs[0], 0, 8), fsize = 4 << fc;
> + uint32_t sc = extract32(env->regs[0], 8, 8), ssize = 1 << sc;
Please put these in separate lines.
> uint64_t pl = get_address(env, 1) & -16;
> uint64_t svh, svl;
> uint32_t cc;
> @@ -1442,7 +1441,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t
> r3, uint64_t a1,
> }
>
> /* Sanity check the alignments. */
> - if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
> + if (extract32(a1, 0, fsize) || extract32(a2, 0, ssize)) {
> goto spec_exception;
> }
>
> @@ -1456,13 +1455,12 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t
> r3, uint64_t a1,
> context in order to implement this. That said, restart early if we
> can't
> support either operation that is supposed to be atomic. */
> if (parallel) {
> - int mask = 0;
> -#if !defined(CONFIG_ATOMIC64)
> - mask = -8;
> -#elif !defined(CONFIG_ATOMIC128)
> - mask = -16;
> + uint32_t max = 4;
> +#ifdef CONFIG_ATOMIC64
> + max = 8;
> #endif
> - if (((4 << fc) | (1 << sc)) & mask) {
> + if ((HAVE_CMPXCHG128 ? 0 : fsize > max) ||
> + (HAVE_ATOMIC128 ? 0 : ssize > max)) {
I don't know what fsize/ssize are, so this is hard to review
for me--just opened the PoO for the first time ever, and I'm
even more confused :-)
The rest of the patch looks good though; the HAVE_ macros
really help keep the code neat.
Emilio
- [Qemu-devel] [PATCH 0/5] tcg: Reorg 128-bit atomic operations, Richard Henderson, 2018/08/15
- [Qemu-devel] [PATCH 2/5] target/i386: Convert to HAVE_CMPXCHG128, Richard Henderson, 2018/08/15
- [Qemu-devel] [PATCH 3/5] target/s390x: Convert to HAVE_CMPXCHG128 and HAVE_ATOMIC128, Richard Henderson, 2018/08/15
- Re: [Qemu-devel] [PATCH 3/5] target/s390x: Convert to HAVE_CMPXCHG128 and HAVE_ATOMIC128,
Emilio G. Cota <=
- [Qemu-devel] [PATCH 5/5] target/ppc: Convert to HAVE_CMPXCHG128 and HAVE_ATOMIC128, Richard Henderson, 2018/08/15
- [Qemu-devel] [PATCH 1/5] tcg: Split CONFIG_ATOMIC128, Richard Henderson, 2018/08/15
- [Qemu-devel] [PATCH 4/5] target/arm: Convert to HAVE_CMPXCHG128, Richard Henderson, 2018/08/15
- Re: [Qemu-devel] [PATCH 0/5] tcg: Reorg 128-bit atomic operations, no-reply, 2018/08/17