Re: [RFC PATCH 05/11] target/ppc: introduce need_addrswizzle_le() function

[Richard Henderson]

On 12/12/24 09:14, Mark Cave-Ayland wrote:
> This function determines whether the MSR_LE bit should be used to implement
> little endian accesses using address swizzling, instead of reversing the
> byte order.
>
> (FIXME: which CPUs?)
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/ppc/translate.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 4c47f97607..1211435039 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -208,6 +208,12 @@ struct DisasContext {
>   #define DISAS_CHAIN        DISAS_TARGET_2  /* lookup next tb, pc updated */
>   #define DISAS_CHAIN_UPDATE DISAS_TARGET_3  /* lookup next tb, pc stale */
>   
> +/* Return true iff address swizzling required */

I've been scolded before for using the iff contraction.
It confuses non-english speakers for little savings.

>   static void gen_ld_tl(DisasContext *ctx, TCGv val, TCGv addr, TCGArg idx,
>                         MemOp memop)
>   {
> -    tcg_gen_qemu_ld_tl(val, addr, idx, memop);
> +    if (!need_addrswizzle_le(ctx)) {
> +        tcg_gen_qemu_ld_tl(val, addr, idx, memop);
> +    }
>   }
>   
>   #define GEN_QEMU_LOAD_TL(ldop, op)                                      \
> @@ -2619,7 +2627,9 @@ GEN_QEMU_LOAD_64(ld64ur, BSWAP_MEMOP(MO_UQ))
>   static void gen_st_tl(DisasContext *ctx, TCGv val, TCGv addr, TCGArg idx,
>                         MemOp memop)
>   {
> -    tcg_gen_qemu_st_tl(val, addr, idx, memop);
> +    if (!need_addrswizzle_le(ctx)) {
> +        tcg_gen_qemu_st_tl(val, addr, idx, memop);
> +    }

These are wrong, in that the load/store are omitted entirely.
Something is missing, like you split the patches incorrectly.


r~