Re: [PATCH v2 28/28] cputlb: Expand cpu_ldst_template.h in cputlb.c

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> Reduce the amount of preprocessor obfuscation by expanding
> the text of each of the functions generated.  The result is
> only slightly smaller than the original.
>
> Reviewed-by: Aleksandar Markovic <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>

\o/

Reviewed-by: Alex Bennée <address@hidden>

> ---
>  include/exec/cpu_ldst.h          |  67 +++++++-----------
>  include/exec/cpu_ldst_template.h | 117 -------------------------------
>  accel/tcg/cputlb.c               | 107 +++++++++++++++++++++++++++-
>  3 files changed, 130 insertions(+), 161 deletions(-)
>  delete mode 100644 include/exec/cpu_ldst_template.h
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index cf4652bf48..62f38d5a22 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -94,32 +94,6 @@ typedef target_ulong abi_ptr;
>  #define TARGET_ABI_FMT_ptr TARGET_ABI_FMT_lx
>  #endif
>  
> -#if defined(CONFIG_USER_ONLY)
> -
> -extern __thread uintptr_t helper_retaddr;
> -
> -static inline void set_helper_retaddr(uintptr_t ra)
> -{
> -    helper_retaddr = ra;
> -    /*
> -     * Ensure that this write is visible to the SIGSEGV handler that
> -     * may be invoked due to a subsequent invalid memory operation.
> -     */
> -    signal_barrier();
> -}
> -
> -static inline void clear_helper_retaddr(void)
> -{
> -    /*
> -     * Ensure that previous memory operations have succeeded before
> -     * removing the data visible to the signal handler.
> -     */
> -    signal_barrier();
> -    helper_retaddr = 0;
> -}
> -
> -/* In user-only mode we provide only the _code and _data accessors. */
> -
>  uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
>  uint32_t cpu_lduw_data(CPUArchState *env, abi_ptr ptr);
>  uint32_t cpu_ldl_data(CPUArchState *env, abi_ptr ptr);
> @@ -148,6 +122,30 @@ void cpu_stl_data_ra(CPUArchState *env, abi_ptr ptr,
>  void cpu_stq_data_ra(CPUArchState *env, abi_ptr ptr,
>                       uint64_t val, uintptr_t retaddr);
>  
> +#if defined(CONFIG_USER_ONLY)
> +
> +extern __thread uintptr_t helper_retaddr;
> +
> +static inline void set_helper_retaddr(uintptr_t ra)
> +{
> +    helper_retaddr = ra;
> +    /*
> +     * Ensure that this write is visible to the SIGSEGV handler that
> +     * may be invoked due to a subsequent invalid memory operation.
> +     */
> +    signal_barrier();
> +}
> +
> +static inline void clear_helper_retaddr(void)
> +{
> +    /*
> +     * Ensure that previous memory operations have succeeded before
> +     * removing the data visible to the signal handler.
> +     */
> +    signal_barrier();
> +    helper_retaddr = 0;
> +}
> +
>  /*
>   * Provide the same *_mmuidx_ra interface as for softmmu.
>   * The mmu_idx argument is ignored.
> @@ -271,23 +269,6 @@ void cpu_stl_mmuidx_ra(CPUArchState *env, abi_ptr addr, 
> uint32_t val,
>  void cpu_stq_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint64_t val,
>                         int mmu_idx, uintptr_t retaddr);
>  
> -/* these access are slower, they must be as rare as possible */
> -#define CPU_MMU_INDEX (cpu_mmu_index(env, false))
> -#define MEMSUFFIX _data
> -#define DATA_SIZE 1
> -#include "exec/cpu_ldst_template.h"
> -
> -#define DATA_SIZE 2
> -#include "exec/cpu_ldst_template.h"
> -
> -#define DATA_SIZE 4
> -#include "exec/cpu_ldst_template.h"
> -
> -#define DATA_SIZE 8
> -#include "exec/cpu_ldst_template.h"
> -#undef CPU_MMU_INDEX
> -#undef MEMSUFFIX
> -
>  #endif /* defined(CONFIG_USER_ONLY) */
>  
>  uint32_t cpu_ldub_code(CPUArchState *env, abi_ptr addr);
> diff --git a/include/exec/cpu_ldst_template.h 
> b/include/exec/cpu_ldst_template.h
> deleted file mode 100644
> index e400979f23..0000000000
> --- a/include/exec/cpu_ldst_template.h
> +++ /dev/null
> @@ -1,117 +0,0 @@
> -/*
> - *  Software MMU support
> - *
> - * Generate inline load/store functions for one MMU mode and data
> - * size.
> - *
> - * Generate a store function as well as signed and unsigned loads.
> - *
> - * Not used directly but included from cpu_ldst.h.
> - *
> - *  Copyright (c) 2003 Fabrice Bellard
> - *
> - * This library is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> - *
> - * This library is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>.
> - */
> -
> -#if DATA_SIZE == 8
> -#define SUFFIX q
> -#define USUFFIX q
> -#define DATA_TYPE uint64_t
> -#define SHIFT 3
> -#elif DATA_SIZE == 4
> -#define SUFFIX l
> -#define USUFFIX l
> -#define DATA_TYPE uint32_t
> -#define SHIFT 2
> -#elif DATA_SIZE == 2
> -#define SUFFIX w
> -#define USUFFIX uw
> -#define DATA_TYPE uint16_t
> -#define DATA_STYPE int16_t
> -#define SHIFT 1
> -#elif DATA_SIZE == 1
> -#define SUFFIX b
> -#define USUFFIX ub
> -#define DATA_TYPE uint8_t
> -#define DATA_STYPE int8_t
> -#define SHIFT 0
> -#else
> -#error unsupported data size
> -#endif
> -
> -#if DATA_SIZE == 8
> -#define RES_TYPE uint64_t
> -#else
> -#define RES_TYPE uint32_t
> -#endif
> -
> -/* generic load/store macros */
> -
> -static inline RES_TYPE
> -glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
> -                                                  target_ulong ptr,
> -                                                  uintptr_t retaddr)
> -{
> -    return glue(glue(cpu_ld, USUFFIX), _mmuidx_ra)(env, ptr, CPU_MMU_INDEX,
> -                                                   retaddr);
> -}
> -
> -static inline RES_TYPE
> -glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
> -{
> -    return glue(glue(cpu_ld, USUFFIX), _mmuidx_ra)(env, ptr, CPU_MMU_INDEX, 
> 0);
> -}
> -
> -#if DATA_SIZE <= 2
> -static inline int
> -glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
> -                                                  target_ulong ptr,
> -                                                  uintptr_t retaddr)
> -{
> -    return glue(glue(cpu_lds, SUFFIX), _mmuidx_ra)(env, ptr, CPU_MMU_INDEX,
> -                                                   retaddr);
> -}
> -
> -static inline int
> -glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
> -{
> -    return glue(glue(cpu_lds, SUFFIX), _mmuidx_ra)(env, ptr, CPU_MMU_INDEX, 
> 0);
> -}
> -#endif
> -
> -/* generic store macro */
> -
> -static inline void
> -glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
> -                                                 target_ulong ptr,
> -                                                 RES_TYPE v, uintptr_t 
> retaddr)
> -{
> -    glue(glue(cpu_st, SUFFIX), _mmuidx_ra)(env, ptr, v, CPU_MMU_INDEX,
> -                                           retaddr);
> -}
> -
> -static inline void
> -glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr,
> -                                      RES_TYPE v)
> -{
> -    glue(glue(cpu_st, SUFFIX), _mmuidx_ra)(env, ptr, v, CPU_MMU_INDEX, 0);
> -}
> -
> -#undef RES_TYPE
> -#undef DATA_TYPE
> -#undef DATA_STYPE
> -#undef SUFFIX
> -#undef USUFFIX
> -#undef DATA_SIZE
> -#undef SHIFT
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index f0e4b0aee4..a991ea2964 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -35,7 +35,6 @@
>  #include "qemu/atomic128.h"
>  #include "translate-all.h"
>  #include "trace-root.h"
> -#include "qemu/plugin.h"
>  #include "trace/mem.h"
>  #ifdef CONFIG_PLUGIN
>  #include "qemu/plugin-memory.h"
> @@ -1697,6 +1696,68 @@ uint64_t cpu_ldq_mmuidx_ra(CPUArchState *env, abi_ptr 
> addr,
>                             ? helper_le_ldq_mmu : helper_be_ldq_mmu);
>  }
>  
> +uint32_t cpu_ldub_data_ra(CPUArchState *env, target_ulong ptr,
> +                          uintptr_t retaddr)
> +{
> +    return cpu_ldub_mmuidx_ra(env, ptr, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +int cpu_ldsb_data_ra(CPUArchState *env, target_ulong ptr, uintptr_t retaddr)
> +{
> +    return cpu_ldsb_mmuidx_ra(env, ptr, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +uint32_t cpu_lduw_data_ra(CPUArchState *env, target_ulong ptr,
> +                          uintptr_t retaddr)
> +{
> +    return cpu_lduw_mmuidx_ra(env, ptr, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +int cpu_ldsw_data_ra(CPUArchState *env, target_ulong ptr, uintptr_t retaddr)
> +{
> +    return cpu_ldsw_mmuidx_ra(env, ptr, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +uint32_t cpu_ldl_data_ra(CPUArchState *env, target_ulong ptr, uintptr_t 
> retaddr)
> +{
> +    return cpu_ldl_mmuidx_ra(env, ptr, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +uint64_t cpu_ldq_data_ra(CPUArchState *env, target_ulong ptr, uintptr_t 
> retaddr)
> +{
> +    return cpu_ldq_mmuidx_ra(env, ptr, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +uint32_t cpu_ldub_data(CPUArchState *env, target_ulong ptr)
> +{
> +    return cpu_ldub_data_ra(env, ptr, 0);
> +}
> +
> +int cpu_ldsb_data(CPUArchState *env, target_ulong ptr)
> +{
> +    return cpu_ldsb_data_ra(env, ptr, 0);
> +}
> +
> +uint32_t cpu_lduw_data(CPUArchState *env, target_ulong ptr)
> +{
> +    return cpu_lduw_data_ra(env, ptr, 0);
> +}
> +
> +int cpu_ldsw_data(CPUArchState *env, target_ulong ptr)
> +{
> +    return cpu_ldsw_data_ra(env, ptr, 0);
> +}
> +
> +uint32_t cpu_ldl_data(CPUArchState *env, target_ulong ptr)
> +{
> +    return cpu_ldl_data_ra(env, ptr, 0);
> +}
> +
> +uint64_t cpu_ldq_data(CPUArchState *env, target_ulong ptr)
> +{
> +    return cpu_ldq_data_ra(env, ptr, 0);
> +}
> +
>  /*
>   * Store Helpers
>   */
> @@ -1970,6 +2031,50 @@ void cpu_stq_mmuidx_ra(CPUArchState *env, target_ulong 
> addr, uint64_t val,
>      cpu_store_helper(env, addr, val, mmu_idx, retaddr, MO_TEQ);
>  }
>  
> +void cpu_stb_data_ra(CPUArchState *env, target_ulong ptr,
> +                     uint32_t val, uintptr_t retaddr)
> +{
> +    cpu_stb_mmuidx_ra(env, ptr, val, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +void cpu_stw_data_ra(CPUArchState *env, target_ulong ptr,
> +                     uint32_t val, uintptr_t retaddr)
> +{
> +    cpu_stw_mmuidx_ra(env, ptr, val, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +void cpu_stl_data_ra(CPUArchState *env, target_ulong ptr,
> +                     uint32_t val, uintptr_t retaddr)
> +{
> +    cpu_stl_mmuidx_ra(env, ptr, val, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +void cpu_stq_data_ra(CPUArchState *env, target_ulong ptr,
> +                     uint64_t val, uintptr_t retaddr)
> +{
> +    cpu_stq_mmuidx_ra(env, ptr, val, cpu_mmu_index(env, false), retaddr);
> +}
> +
> +void cpu_stb_data(CPUArchState *env, target_ulong ptr, uint32_t val)
> +{
> +    cpu_stb_data_ra(env, ptr, val, 0);
> +}
> +
> +void cpu_stw_data(CPUArchState *env, target_ulong ptr, uint32_t val)
> +{
> +    cpu_stw_data_ra(env, ptr, val, 0);
> +}
> +
> +void cpu_stl_data(CPUArchState *env, target_ulong ptr, uint32_t val)
> +{
> +    cpu_stl_data_ra(env, ptr, val, 0);
> +}
> +
> +void cpu_stq_data(CPUArchState *env, target_ulong ptr, uint64_t val)
> +{
> +    cpu_stq_data_ra(env, ptr, val, 0);
> +}
> +
>  /* First set of helpers allows passing in of OI and RETADDR.  This makes
>     them callable from other helpers.  */


-- 
Alex Bennée