Re: Behaviour of jit_qrsh()

[Paulo César Pereira de Andrade]

Em seg., 2 de out. de 2023 às 19:46, Paul Cercueil
<paul@crapouillou.net> escreveu:
>
> Hi Paulo,

  Hi Paul,

> Le lundi 02 octobre 2023 à 08:57 -0300, Paulo César Pereira de Andrade
> a écrit :
> > Em qui., 28 de set. de 2023 às 10:20, Paul Cercueil
> > <paul@crapouillou.net> escreveu:
> > >
> > > Hi Paulo,
> >
> >   Hi Paul,
> >
> > > I'm trying to implement the q-shift operations on SH4.
> > >
> > > In the doc, jit_qrsh() is defined as this operation:
> > > O1 = O3 >> O4, O2 = O3 << (WORDSIZE - O4)
> >
> >   I considered writing a "fast" version without special cases. But at
> > the end decided to keep only a long version, with defined behavior
> > for special cases. The special cases are 0 and wordsize shifts.
> > Shift values not in the inclusive 0 ... wordsize range are undefined.
> >
> > > qalu_shift.tst does this test:
> > > QRSH(0, 0x89abcdef, 0, 0x89abcdef, 0xffffffff)
> >
> >   It is just a weird way to sign extend 0x89abcdef with a zero signed
> > shift to right. Essentially simple reproducer is C is:
> > #include <stdio.h>
> > int main() {
> >     int i = 0x89abcdef;
> >     printf("%llx\n", (long long)i);
> >     return 0;
> > }
>
> That would be the behaviour of RSHR #32, which indeed sign-extends the
> value into two registers. For RSHR #0, you left-shift by WORDSIZE bits,
> not right-shift, so O2 should always be 0.

  You are completely right. It was just a wrong idea that I propagated
in all ports, and did not think about its correctness later...

> Besides, for any other value of O4, the behaviour is that the LSBs of
> O3, not the MSBs, will be shifted into O2. So I really don't see a
> reason why O3 would contain the LSBs for all values of O4 but 0, where
> it would contain the sign-extended MSBs. It just doesn't make sense to
> me.
>
> Do you really depend on this behaviour somewhere?

  No. It is a new feature that has not yet been put in use. The idea
is to not lose the shifted bits, and also work as a way to check if a
division is exact. A right shift of zero in this case would be a division
by one.

> > > However, I don't see how this can be valid. "O3 << (32 - 0)" should
> > > always be 0, not 0xffffffff. So I believe  this check should
> > > actually
> > > be:
> > > QRSH(0, 0x89abcdef, 0, 0x89abcdef, 0x00000000)
> >
> >   The unsigned test shows this:
> >
> > QRSHU(0, 0x89abcdef, 0, 0x89abcdef, 0x00000000)
>
> QRSH / QRSHU #0 would be equivalent, yes, but the O0 register would be
> different for any other value, as the former uses an arithmetic right-
> shift while the latter uses a logical right-shift.

  Need to fix check/qalu_shift.tst implementation of func_qrsh and
func_qrsh_u to generate the tables again, and after that fool proof
validate it is correct in all ports.

  I will work on this in the next few days.

> Speaking about tests, qalu_shift.txt only tests with a single value
> 0x89abcdef, it should probably also use a signed positive value for
> signed QRSH / QLSH.

  Doing it for the 0 and wordsize values should be enough. Or add
other values under the OPTIONAL macro.

> Cheers,
> -Paul

Thanks!
Paulo