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

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

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.  */