Re: [PATCH 11/37] accel/tcg: Implement translator_ld*_end

2025-03-14 Thread Richard Henderson 

On 3/13/25 10:33, Pierrick Bouvier wrote:

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*().


This combines the tswap in the core routine with the bswap in 
translator_ld_swap().

It enables cleanup in the various translators where we currently choose "swap from 
TARGET_BIG_ENDIAN" rather than specifying the absolute endianness desired, which is 
usually already at hand for use by all of the other memory references.



r~

Re: [PATCH 11/37] accel/tcg: Implement translator_ld*_end

2025-03-13 Thread Pierrick Bouvier 

On 3/13/25 11:17, Richard Henderson wrote:

On 3/13/25 10:33, Pierrick Bouvier wrote:

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*().


This combines the tswap in the core routine with the bswap in 
translator_ld_swap().

It enables cleanup in the various translators where we currently choose "swap 
from
TARGET_BIG_ENDIAN" rather than specifying the absolute endianness desired, 
which is
usually already at hand for use by all of the other memory references.



Ok.
Reviewed-by: Pierrick Bouvier 



r~

Re: [PATCH 11/37] accel/tcg: Implement translator_ld*_end

2025-03-13 Thread Pierrick Bouvier 

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 
---
  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, size