diff mbox series

[v2,03/28] trace: Remove trace_mem_build_info_no_se_[bl]e

Message ID 20191216221158.29572-4-richard.henderson@linaro.org
State Superseded
Headers show
Series cputlb: Remove support for MMU_MODE*_SUFFIX | expand

Commit Message

Richard Henderson Dec. 16, 2019, 10:11 p.m. UTC 
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 <richard.henderson@linaro.org>

---
 accel/tcg/atomic_template.h | 67 +++++++++++++------------------------
 trace/mem-internal.h        | 17 ----------
 2 files changed, 24 insertions(+), 60 deletions(-)

-- 
2.20.1

Comments

Alex Bennée Dec. 20, 2019, 4:38 p.m. UTC | #1 
Richard Henderson <richard.henderson@linaro.org> 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 <richard.henderson@linaro.org>

> ---

>  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 <alex.bennee@linaro.org>


-- 
Alex Bennée
Richard Henderson Dec. 28, 2019, 8:48 p.m. UTC | #2 
On 12/21/19 3:38 AM, Alex Bennée wrote:
> 

> Richard Henderson <richard.henderson@linaro.org> 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 <richard.henderson@linaro.org>

>> ---

>>  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?


See the first hunk, where MEND is removed.  It's based on HOST_WORDS_BIGENDIAN,
so no we don't use MO_TE.


r~
diff mbox series

Patch

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,
+                                         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);
 
     atomic_trace_rmw_pre(env, addr, info);
 #if DATA_SIZE == 16
@@ -254,9 +243,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, MO_BSWAP, false,
+                                         ATOMIC_MMU_IDX);
 
     atomic_trace_ld_pre(env, addr, info);
     val = atomic16_read(haddr);
@@ -270,9 +258,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, MO_BSWAP, true,
+                                         ATOMIC_MMU_IDX);
 
     val = BSWAP(val);
     atomic_trace_st_pre(env, addr, info);
@@ -289,9 +276,8 @@  ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
     ATOMIC_MMU_DECLS;
     DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
     ABI_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);
 
     atomic_trace_rmw_pre(env, addr, info);
     ret = atomic_xchg__nocheck(haddr, BSWAP(val));
@@ -307,10 +293,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, MO_BSWAP,    \
+                                         false, ATOMIC_MMU_IDX);    \
     atomic_trace_rmw_pre(env, addr, info);                          \
     ret = atomic_##X(haddr, BSWAP(val));                            \
     ATOMIC_MMU_CLEANUP;                                             \
@@ -341,10 +325,8 @@  ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,       \
     ATOMIC_MMU_DECLS;                                               \
     XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;                          \
     XDATA_TYPE ldo, ldn, 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, MO_BSWAP,    \
+                                         false, ATOMIC_MMU_IDX);    \
     atomic_trace_rmw_pre(env, addr, info);                          \
     smp_mb();                                                       \
     ldn = atomic_read__nocheck(haddr);                              \
@@ -378,7 +360,6 @@  GEN_ATOMIC_HELPER_FN(add_fetch, ADD, DATA_TYPE, new)
 #endif /* DATA_SIZE >= 16 */
 
 #undef END
-#undef MEND
 #endif /* DATA_SIZE > 1 */
 
 #undef BSWAP
diff --git a/trace/mem-internal.h b/trace/mem-internal.h
index 0a32aa22ca..8b72b678fa 100644
--- a/trace/mem-internal.h
+++ b/trace/mem-internal.h
@@ -47,21 +47,4 @@  static inline uint16_t trace_mem_get_info(MemOp op,
                                 mmu_idx);
 }
 
-/* Used by the atomic helpers */
-static inline
-uint16_t trace_mem_build_info_no_se_be(int size_shift, bool store,
-                                       TCGMemOpIdx oi)
-{
-    return trace_mem_build_info(size_shift, false, MO_BE, store,
-                                get_mmuidx(oi));
-}
-
-static inline
-uint16_t trace_mem_build_info_no_se_le(int size_shift, bool store,
-                                       TCGMemOpIdx oi)
-{
-    return trace_mem_build_info(size_shift, false, MO_LE, store,
-                                get_mmuidx(oi));
-}
-
 #endif /* TRACE__MEM_INTERNAL_H */