Re: [PATCH 4/8] target/ppc: Evaluate TARGET_BIG_ENDIAN at compile time

[Richard Henderson]

On 4/17/25 06:10, Philippe Mathieu-Daudé wrote:

  static inline bool needs_byteswap(const CPUPPCState *env)
  {
-#if TARGET_BIG_ENDIAN
-  return FIELD_EX64(env->msr, MSR, LE);
-#else
-  return !FIELD_EX64(env->msr, MSR, LE);
-#endif
+  return TARGET_BIG_ENDIAN ^ FIELD_EX64(env->msr, MSR, LE);
  }


This is wrong.  You wanted

   !TARGET_BIG_ENDIAN ^ ...

Likewise in the other instance in translate.c.


r~

Re: [PATCH 4/8] target/ppc: Evaluate TARGET_BIG_ENDIAN at compile time

[Pierrick Bouvier]

On 4/17/25 06:10, Philippe Mathieu-Daudé wrote:

Rather than evaluating TARGET_BIG_ENDIAN at preprocessing
time via #ifdef'ry, do it in C at compile time

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/ppc/cpu_init.c   | 12 ++--
  target/ppc/mem_helper.c |  6 +-
  target/ppc/translate.c  |  6 +-
  3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 077991ed535..bbab411a07c 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7262,14 +7262,14 @@ static void ppc_cpu_reset_hold(Object *obj, ResetType 
type)
  #if defined(TARGET_PPC64)
  msr |= (target_ulong)1 << MSR_TM; /* Transactional memory */
  #endif
-#if !TARGET_BIG_ENDIAN
-msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
-if (!((env->msr_mask >> MSR_LE) & 1)) {
-fprintf(stderr, "Selected CPU does not support little-endian.\n");
-exit(1);
+if (!TARGET_BIG_ENDIAN) {
+msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
+if (!((env->msr_mask >> MSR_LE) & 1)) {
+fprintf(stderr, "Selected CPU does not support little-endian.\n");
+exit(1);
+}
  }
  #endif
-#endif
  
  #if defined(TARGET_PPC64)

  if (mmu_is_64bit(env->mmu_model)) {
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index d7e8d678f4b..cc3ed29a35b 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -32,11 +32,7 @@
  
  static inline bool needs_byteswap(const CPUPPCState *env)

  {
-#if TARGET_BIG_ENDIAN
-  return FIELD_EX64(env->msr, MSR, LE);
-#else
-  return !FIELD_EX64(env->msr, MSR, LE);
-#endif
+  return TARGET_BIG_ENDIAN ^ FIELD_EX64(env->msr, MSR, LE);
  }
  
  /*/

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 399107d319a..828b850b40e 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -213,11 +213,7 @@ struct DisasContext {
  /* Return true iff byteswap is needed in a scalar memop */
  static inline bool need_byteswap(const DisasContext *ctx)
  {
-#if TARGET_BIG_ENDIAN
- return ctx->le_mode;
-#else
- return !ctx->le_mode;
-#endif
+ return TARGET_BIG_ENDIAN ^ ctx->le_mode;


Maybe I'm missing something, but with {TARGET_BIG_ENDIAN: 1, 
ctx->le_mode: 1}, it would return 0, while it should be 1 instead.


It would be more readable to use a if or a ternary expression.
Same for previous function needs_byteswap.


  }
  
  /* True when active word size < size of target_long.  */