misc: Prefer evaluating TARGET_BIG_ENDIAN in C

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

Posted by Philippe Mathieu-Daudé 7 months ago

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é <philmd@linaro.org>
---
 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;
 }
 
 /* True when active word size < size of target_long.  */
-- 
2.47.1

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

Posted by Richard Henderson 7 months ago

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

Posted by Pierrick Bouvier 7 months ago

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é <philmd@linaro.org>
> ---
>   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.  */