Re: [PATCH-for-11.0 v3 11/22] target/mips: Use big-endian variant of cpu_ld/st_data*() for MSA opcode

[Richard Henderson <richard.henderson@linaro.org>]

On 11/26/25 14:21, Philippe Mathieu-Daudé wrote:
> MSA vectors are accessed in big endianness.
> 
> Per the "MIPS® SIMD Architecture" (MD00926 rev 1.03):
> 
>    3.1 Registers Layout
> 
>    MSA vectors are stored in memory starting from the 0th element at
>    the lowest byte address. The byte order of each element follows the
>    big- or little-endian convention of the system configuration.

This says "follow the system configuration" ...

> 
> Use the explicit big-endian variants of cpu_ld/st_data*().

... so how do you get "big-endian" from that?

> diff --git a/target/mips/tcg/msa_helper.c b/target/mips/tcg/msa_helper.c
> index f554b3d10ee..d6ce17abf9a 100644
> --- a/target/mips/tcg/msa_helper.c
> +++ b/target/mips/tcg/msa_helper.c
> @@ -8231,8 +8231,8 @@ void helper_msa_ld_b(CPUMIPSState *env, uint32_t wd,
>       uint64_t d0, d1;
>   
>       /* Load 8 bytes at a time.  Vector element ordering makes this LE.  */
> -    d0 = cpu_ldq_le_data_ra(env, addr + 0, ra);
> -    d1 = cpu_ldq_le_data_ra(env, addr + 8, ra);
> +    d0 = cpu_ldq_be_data_ra(env, addr + 0, ra);
> +    d1 = cpu_ldq_be_data_ra(env, addr + 8, ra);
>       pwd->d[0] = d0;
>       pwd->d[1] = d1;

This really seems to be exchanging one bug for another.
And the comment no longer matches.

Also, this would be much better accomplished inline as

     tcg_gen_ld_i128(..., mo_endian(s) | MO_128 | MO_ATOM_<something>);

where <something> will be MO_ATOM_NONE for vector of 1-byte elements, MO_ATOM_IFALIGN_PAIR 
for 8-byte elements, and MO_ATOM_SUBALIGN for the others (which is slightly stronger than 
required, but we don't have MO_ATOM_IFALIGN_<N> for all N).

Doing that for the stores as well will allow removal of ...

>       ensure_writable_pages(env, addr, mmu_idx, GETPC());

... this hack.


r~