On 3/12/25 20:44, Richard Henderson wrote:
> Add a new family of translator load functions which take
> an absolute endianness value in the form of MO_BE/MO_LE.
> Expand the other translator_ld* functions on top of this.
> Remove exec/tswap.h from translator.c.
>
Is there a need further down the road to break the dependency to tswap?
I am not sure of the benefit to drop tswap, as the resulting code is
more complicated than simply calling tswap*().
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/translator.h | 49 ++++++++++++++++++++++++---------------
> accel/tcg/translator.c | 26 +++++++++++++++------
> 2 files changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 205dd85bba..3c32655569 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -18,7 +18,7 @@
> * member in your target-specific DisasContext.
> */
>
> -#include "qemu/bswap.h"
> +#include "exec/memop.h"
> #include "exec/vaddr.h"
>
> /**
> @@ -181,42 +181,53 @@ bool translator_io_start(DisasContextBase *db);
> */
>
> uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc);
> -uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc);
> -uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc);
> -uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc);
> +uint16_t translator_lduw_end(CPUArchState *env, DisasContextBase *db,
> + vaddr pc, MemOp endian);
> +uint32_t translator_ldl_end(CPUArchState *env, DisasContextBase *db,
> + vaddr pc, MemOp endian);
> +uint64_t translator_ldq_end(CPUArchState *env, DisasContextBase *db,
> + vaddr pc, MemOp endian);
> +
> +#ifdef COMPILING_PER_TARGET
> +static inline uint16_t
> +translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
> +{
> + return translator_lduw_end(env, db, pc, MO_TE);
> +}
> +
> +static inline uint32_t
> +translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
> +{
> + return translator_ldl_end(env, db, pc, MO_TE);
> +}
> +
> +static inline uint64_t
> +translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
> +{
> + return translator_ldq_end(env, db, pc, MO_TE);
> +}
>
> static inline uint16_t
> translator_lduw_swap(CPUArchState *env, DisasContextBase *db,
> vaddr pc, bool do_swap)
> {
> - uint16_t ret = translator_lduw(env, db, pc);
> - if (do_swap) {
> - ret = bswap16(ret);
> - }
> - return ret;
> + return translator_lduw_end(env, db, pc, MO_TE ^ (do_swap * MO_BSWAP));
> }
>
> static inline uint32_t
> translator_ldl_swap(CPUArchState *env, DisasContextBase *db,
> vaddr pc, bool do_swap)
> {
> - uint32_t ret = translator_ldl(env, db, pc);
> - if (do_swap) {
> - ret = bswap32(ret);
> - }
> - return ret;
> + return translator_ldl_end(env, db, pc, MO_TE ^ (do_swap * MO_BSWAP));
> }
>
> static inline uint64_t
> translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
> vaddr pc, bool do_swap)
> {
> - uint64_t ret = translator_ldq(env, db, pc);
> - if (do_swap) {
> - ret = bswap64(ret);
> - }
> - return ret;
> + return translator_ldq_end(env, db, pc, MO_TE ^ (do_swap * MO_BSWAP));
> }
> +#endif /* COMPILING_PER_TARGET */
>
> /**
> * translator_fake_ld - fake instruction load
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 64fa069b51..405e0b44c4 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -8,13 +8,13 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/bswap.h"
> #include "qemu/log.h"
> #include "qemu/error-report.h"
> #include "exec/exec-all.h"
> #include "exec/cpu-ldst-common.h"
> #include "exec/translator.h"
> #include "exec/plugin-gen.h"
> -#include "exec/tswap.h"
> #include "tcg/tcg-op-common.h"
> #include "internal-target.h"
> #include "disas/disas.h"
> @@ -467,7 +467,8 @@ uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc)
> return val;
> }
>
> -uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
> +uint16_t translator_lduw_end(CPUArchState *env, DisasContextBase *db,
> + vaddr pc, MemOp endian)
> {
> uint16_t val;
>
> @@ -476,10 +477,14 @@ uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
> val = cpu_ldw_code_mmu(env, pc, oi, 0);
> record_save(db, pc, &val, sizeof(val));
> }
> - return tswap16(val);
> + if (endian & MO_BSWAP) {
> + val = bswap16(val);
> + }
> + return val;
> }
>
> -uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
> +uint32_t translator_ldl_end(CPUArchState *env, DisasContextBase *db,
> + vaddr pc, MemOp endian)
> {
> uint32_t val;
>
> @@ -488,10 +493,14 @@ uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
> val = cpu_ldl_code_mmu(env, pc, oi, 0);
> record_save(db, pc, &val, sizeof(val));
> }
> - return tswap32(val);
> + if (endian & MO_BSWAP) {
> + val = bswap32(val);
> + }
> + return val;
> }
>
> -uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
> +uint64_t translator_ldq_end(CPUArchState *env, DisasContextBase *db,
> + vaddr pc, MemOp endian)
> {
> uint64_t val;
>
> @@ -500,7 +509,10 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
> val = cpu_ldq_code_mmu(env, pc, oi, 0);
> record_save(db, pc, &val, sizeof(val));
> }
> - return tswap64(val);
> + if (endian & MO_BSWAP) {
> + val = bswap64(val);
> + }
> + return val;
> }
>
> void translator_fake_ld(DisasContextBase *db, const void *data, size_t len)