[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 03/28] trace: Remove trace_mem_build_info_no_se_[bl]e
|
From: |
Alex Bennée |
|
Subject: |
Re: [PATCH v2 03/28] trace: Remove trace_mem_build_info_no_se_[bl]e |
|
Date: |
Fri, 20 Dec 2019 16:38:02 +0000 |
|
User-agent: |
mu4e 1.3.5; emacs 27.0.50 |
Richard Henderson <address@hidden> writes:
> It is easy for the atomic helpers to use trace_mem_build_info
> directly, without resorting to symbol pasting. For this usage,
> we cannot use trace_mem_get_info, because the MemOp does not
> support 16-byte accesses.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> accel/tcg/atomic_template.h | 67 +++++++++++++------------------------
> trace/mem-internal.h | 17 ----------
> 2 files changed, 24 insertions(+), 60 deletions(-)
>
> diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
> index 837676231f..26969487d6 100644
> --- a/accel/tcg/atomic_template.h
> +++ b/accel/tcg/atomic_template.h
> @@ -64,13 +64,10 @@
> the ATOMIC_NAME macro, and redefined below. */
> #if DATA_SIZE == 1
> # define END
> -# define MEND _be /* either le or be would be fine */
> #elif defined(HOST_WORDS_BIGENDIAN)
> # define END _be
> -# define MEND _be
> #else
> # define END _le
> -# define MEND _le
> #endif
>
> ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
> @@ -79,8 +76,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env,
> target_ulong addr,
> ATOMIC_MMU_DECLS;
> DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> DATA_TYPE ret;
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false,
> - ATOMIC_MMU_IDX);
> + uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
What is MEND meant to be? Shouldn't we use the appropriate MO_TE instead
of 0 for these helpers?
> + ATOMIC_MMU_IDX);
>
> atomic_trace_rmw_pre(env, addr, info);
> #if DATA_SIZE == 16
> @@ -99,8 +96,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong
> addr EXTRA_ARGS)
> {
> ATOMIC_MMU_DECLS;
> DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP;
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false,
> - ATOMIC_MMU_IDX);
> + uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
> + ATOMIC_MMU_IDX);
>
> atomic_trace_ld_pre(env, addr, info);
> val = atomic16_read(haddr);
> @@ -114,8 +111,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
> {
> ATOMIC_MMU_DECLS;
> DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, true,
> - ATOMIC_MMU_IDX);
> + uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
> + ATOMIC_MMU_IDX);
>
> atomic_trace_st_pre(env, addr, info);
> atomic16_set(haddr, val);
> @@ -130,8 +127,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env,
> target_ulong addr,
> ATOMIC_MMU_DECLS;
> DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> DATA_TYPE ret;
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false,
> - ATOMIC_MMU_IDX);
> + uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
> + ATOMIC_MMU_IDX);
>
> atomic_trace_rmw_pre(env, addr, info);
> ret = atomic_xchg__nocheck(haddr, val);
> @@ -147,10 +144,8 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong
> addr, \
> ATOMIC_MMU_DECLS; \
> DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \
> DATA_TYPE ret; \
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, \
> - false, \
> - ATOMIC_MMU_IDX); \
> - \
> + uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
> + ATOMIC_MMU_IDX); \
> atomic_trace_rmw_pre(env, addr, info); \
> ret = atomic_##X(haddr, val); \
> ATOMIC_MMU_CLEANUP; \
> @@ -183,10 +178,8 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong
> addr, \
> ATOMIC_MMU_DECLS; \
> XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \
> XDATA_TYPE cmp, old, new, val = xval; \
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, \
> - false, \
> - ATOMIC_MMU_IDX); \
> - \
> + uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
> + ATOMIC_MMU_IDX); \
> atomic_trace_rmw_pre(env, addr, info); \
> smp_mb(); \
> cmp = atomic_read__nocheck(haddr); \
> @@ -213,7 +206,6 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX, DATA_TYPE, new)
> #endif /* DATA SIZE >= 16 */
>
> #undef END
> -#undef MEND
>
> #if DATA_SIZE > 1
>
> @@ -221,10 +213,8 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX, DATA_TYPE, new)
> within the ATOMIC_NAME macro. */
> #ifdef HOST_WORDS_BIGENDIAN
> # define END _le
> -# define MEND _le
> #else
> # define END _be
> -# define MEND _be
> #endif
>
> ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
> @@ -233,9 +223,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env,
> target_ulong addr,
> ATOMIC_MMU_DECLS;
> DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> DATA_TYPE ret;
> - uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT,
> - false,
> - ATOMIC_MMU_IDX);
> + uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
> + ATOMIC_MMU_IDX);
These are fine with MO_BSWAP. So otherwise:
Reviewed-by: Alex Bennée <address@hidden>
--
Alex Bennée
- [PATCH v2 00/28] cputlb: Remove support for MMU_MODE*_SUFFIX, Richard Henderson, 2019/12/16
- [PATCH v2 01/28] target/xtensa: Use probe_access for itlb_hit_test, Richard Henderson, 2019/12/16
- [PATCH v2 02/28] cputlb: Use trace_mem_get_info instead of trace_mem_build_info, Richard Henderson, 2019/12/16
- [PATCH v2 03/28] trace: Remove trace_mem_build_info_no_se_[bl]e, Richard Henderson, 2019/12/16
- Re: [PATCH v2 03/28] trace: Remove trace_mem_build_info_no_se_[bl]e,
Alex Bennée <=
- [PATCH v2 04/28] cputlb: Move body of cpu_ldst_template.h out of line, Richard Henderson, 2019/12/16
- [PATCH v2 05/28] translator: Use cpu_ld*_code instead of open-coding, Richard Henderson, 2019/12/16
- [PATCH v2 06/28] cputlb: Rename helper_ret_ld*_cmmu to cpu_ld*_code, Richard Henderson, 2019/12/16