[v5] target/ppc: Remove most MO_TE uses in cpu_ld/st_code/data()

[PATCH v5 06/11] target/ppc: Introduce ppc_code_endian() helper

Posted by Philippe Mathieu-Daudé 2 weeks, 4 days ago

Introduce the ppc_code_endian() helper which returns the
MemOp endianness for the CODE path.

Use it in need_byteswap(), removing one TARGET_BIG_ENDIAN.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/translate.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 17e6d07c8c2..4a9199a4473 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -214,14 +214,15 @@ static inline bool is_ppe(const DisasContext *ctx)
     return !!(ctx->flags & POWERPC_FLAG_PPE42);
 }
 
+static inline MemOp ppc_code_endian(const DisasContext *ctx)
+{
+    return MO_BE ^ (ctx->le_mode * MO_BSWAP);
+}
+
 /* 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 ppc_code_endian(ctx) != MO_TE;
 }
 
 /* True when active word size < size of target_long.  */
-- 
2.52.0

Re: [PATCH v5 06/11] target/ppc: Introduce ppc_code_endian() helper

Posted by Anton Johansson via qemu development 1 week ago

On 22/01/26, Philippe Mathieu-Daudé wrote:
> Introduce the ppc_code_endian() helper which returns the
> MemOp endianness for the CODE path.
> 
> Use it in need_byteswap(), removing one TARGET_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/ppc/translate.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 17e6d07c8c2..4a9199a4473 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -214,14 +214,15 @@ static inline bool is_ppe(const DisasContext *ctx)
>      return !!(ctx->flags & POWERPC_FLAG_PPE42);
>  }
>  
> +static inline MemOp ppc_code_endian(const DisasContext *ctx)
> +{
> +    return MO_BE ^ (ctx->le_mode * MO_BSWAP);
> +}

This was not super obvious to me, is this a common pattern?

Either way the behaviour is correct:

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH v5 06/11] target/ppc: Introduce ppc_code_endian() helper

Posted by Anton Johansson via qemu development 1 week ago

On 02/02/26, Anton Johansson wrote:
> On 22/01/26, Philippe Mathieu-Daudé wrote:
> > Introduce the ppc_code_endian() helper which returns the
> > MemOp endianness for the CODE path.
> > 
> > Use it in need_byteswap(), removing one TARGET_BIG_ENDIAN.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >  target/ppc/translate.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > index 17e6d07c8c2..4a9199a4473 100644
> > --- a/target/ppc/translate.c
> > +++ b/target/ppc/translate.c
> > @@ -214,14 +214,15 @@ static inline bool is_ppe(const DisasContext *ctx)
> >      return !!(ctx->flags & POWERPC_FLAG_PPE42);
> >  }
> >  
> > +static inline MemOp ppc_code_endian(const DisasContext *ctx)
> > +{
> > +    return MO_BE ^ (ctx->le_mode * MO_BSWAP);
> > +}
> 
> This was not super obvious to me, is this a common pattern?

Ignore this comment, I see we do it for translator_ld*()!:)

Re: [PATCH v5 06/11] target/ppc: Introduce ppc_code_endian() helper

Posted by Pierrick Bouvier 1 week ago

On 2/2/26 6:39 AM, Anton Johansson wrote:
> On 02/02/26, Anton Johansson wrote:
>> On 22/01/26, Philippe Mathieu-Daudé wrote:
>>> Introduce the ppc_code_endian() helper which returns the
>>> MemOp endianness for the CODE path.
>>>
>>> Use it in need_byteswap(), removing one TARGET_BIG_ENDIAN.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   target/ppc/translate.c | 11 ++++++-----
>>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>>> index 17e6d07c8c2..4a9199a4473 100644
>>> --- a/target/ppc/translate.c
>>> +++ b/target/ppc/translate.c
>>> @@ -214,14 +214,15 @@ static inline bool is_ppe(const DisasContext *ctx)
>>>       return !!(ctx->flags & POWERPC_FLAG_PPE42);
>>>   }
>>>   
>>> +static inline MemOp ppc_code_endian(const DisasContext *ctx)
>>> +{
>>> +    return MO_BE ^ (ctx->le_mode * MO_BSWAP);
>>> +}
>>
>> This was not super obvious to me, is this a common pattern?
> 
> Ignore this comment, I see we do it for translator_ld*()!:)

I agree that even though it's used there, it's far from being obvious 
when reading it :)

Re: [PATCH v5 06/11] target/ppc: Introduce ppc_code_endian() helper

Posted by Philippe Mathieu-Daudé 1 week ago

On 2/2/26 18:56, Pierrick Bouvier wrote:
> On 2/2/26 6:39 AM, Anton Johansson wrote:
>> On 02/02/26, Anton Johansson wrote:
>>> On 22/01/26, Philippe Mathieu-Daudé wrote:
>>>> Introduce the ppc_code_endian() helper which returns the
>>>> MemOp endianness for the CODE path.
>>>>
>>>> Use it in need_byteswap(), removing one TARGET_BIG_ENDIAN.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>   target/ppc/translate.c | 11 ++++++-----
>>>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>>>> index 17e6d07c8c2..4a9199a4473 100644
>>>> --- a/target/ppc/translate.c
>>>> +++ b/target/ppc/translate.c
>>>> @@ -214,14 +214,15 @@ static inline bool is_ppe(const DisasContext 
>>>> *ctx)
>>>>       return !!(ctx->flags & POWERPC_FLAG_PPE42);
>>>>   }
>>>> +static inline MemOp ppc_code_endian(const DisasContext *ctx)
>>>> +{
>>>> +    return MO_BE ^ (ctx->le_mode * MO_BSWAP);
>>>> +}
>>>
>>> This was not super obvious to me, is this a common pattern?
>>
>> Ignore this comment, I see we do it for translator_ld*()!:)
> 
> I agree that even though it's used there, it's far from being obvious 
> when reading it :)

In order to clarify, I amended:

---
     Note, the target MemOp endianness can be evaluated as (see
     commit 5c43a750b67 "accel/tcg: Implement translator_ld*_end"):

         MO_TE ^ (do_swap * MO_BSWAP)

     For PPC we use the DisasContext::le_mode field to swap the
     default (big-endian) order, so to get the PPC MemOp endianness
     we can directly use:

         MO_BE ^ (ctx->le_mode * MO_BSWAP)
---