Re: [Qemu-devel] [PATCH 2/4] target/mips: Fix copy_s.<b|h|w> for MIPS big endian host

[Mateja Marjanovic]

On 22.3.19. 18:45, Aleksandar Markovic wrote:
> > From: Mateja Marjanovic <address@hidden>
> > Subject: [PATCH 2/4] target/mips: Fix copy_s.<b|h|w> for MIPS big endian host
> >
> > From: Mateja Marjanovic <address@hidden>
> >
> > Signed element copy from MSA registers to GPR when
> > executed on a MIPS big endian CPU, didn't pick the
> > right element, and was behaving like on little endian.
> >
> > Signed-off-by: Mateja Marjanovic <address@hidden>
> > ---
>  From the title, I gather you are testing on a 32-bit big-endian
> host. (If you had tested on a 64-bit host, you would most likely
> had said "<b|h|w|d>" rather than <b|h|w>.) This means you can
> test only MIPS32 MSA, since QEMU can't emulate a 64-bit target
> system on a 32-bit host.
>
> But what about COPY_S.D? This instruction is present only in
> MIPS64 MSA. If you don't have access to the 64-bit big-endian
> host, could you perhaps do some logical analysis of handling
> of COPY_S.D on big-endian host?
Well, in the case of doubleword, there are only two registers, and
each has only one element,  unlike in the case of halfword (or word,
or byte), where the number of elements is 4 (2, 8) per register.
The difference between big endian and litlle endian is that copying
from the 0th element in little endian, is the same as copying from
the 3rd in big endian (but still in same register). Likewise, copying
from the 4th element in LE is the same as copying from the 7th
element in BE. So, if we had a number, let's say
FA87 1BC2 FED2 0411 D16B C014 BFF0 01E1
and we wanted to copy (COPY_S.H) the 1st element into the gpr,
big endian would copy 0xBFF0 into the grp, and little endian would
copy 0xC014 into the gpr (because the contents of the lower register
are D16B C014 BFF0 01E1).
 LE     0        1       2       3
 BE    3        2       1       0
> An almost same question arises for the next patch of course,
> regarding COPY_U.D.
I am not sure that COPY_U.D exists. One of the next patches will
correct that in target/mips/msa_helper.c and target/mips/translate.c
> Sincerely,
> Aleksandar
Regards,
Mateja
> >   target/mips/msa_helper.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
> > index 421dced..012f373 100644
> > --- a/target/mips/msa_helper.c
> > +++ b/target/mips/msa_helper.c
> > @@ -1435,6 +1435,13 @@ void helper_msa_copy_s_df(CPUMIPSState *env, uint32_t 
> > df, uint32_t rd,
> >                             uint32_t ws, uint32_t n)
> >   {
> >       n %= DF_ELEMENTS(df);
> > +#if defined(HOST_WORDS_BIGENDIAN)
> > +    if (n < DF_ELEMENTS(df) / 2) {
> > +        n = DF_ELEMENTS(df) / 2 - n - 1;
> > +    } else {
> > +        n = 3 * DF_ELEMENTS(df) / 2 - n - 1;
> > +    }
> > +#endif
> >
> >       switch (df) {
> >       case DF_BYTE: