Re: [9front] Weird >>= operator behaviour in 5c/5l

[Jonas Amoson <jonas.amoson@gmail.com>]

Great!
I have not tested your patch extensively yet, but Netsurf now runs
without graphical glitches on 32-bit arm on my old Rpi terminal.
Jonas

On Wed, 20 Jan 2021 at 08:39, <ori@eigenstate.org> wrote:
>
> Quoth ori@eigenstate.org:
> > Quoth Jonas Amoson <jonas.amoson@gmail.com>:
> > > Left shifting by 32 instead of 0 does explain my
> > > result, and seam indeed to be the real bug.
> > >
> > > Regarding optimising out shifts where the number
> > > of steps is zero: it already does that when using the
> > > pattern x = x >> 0; but it doesn't for x >>= 0;
> > >
> > > Thanks,
> > > Jonas
> > >
> > > On Mon, 18 Jan 2021 at 07:50, <ori@eigenstate.org> wrote:
> > > >
> > > > Quoth Noam Preil <noam@pixelhero.dev>:
> > > > > It could just be architecture specific behavior.
> > > > >
> > > >
> > > > It's a bug. Two, actually.
> > > >
> > > > 1) a shift of zero should be optimized out by
> > > >    the compiler; cinap has a change for this in
> > > >    the works already. This fix should hide the
> > > >    second bug.
> > > >
> > > > 2) a shift by a literal zero is miscompiled by
> > > >    the assembler into a shift by 32, because
> > > >    of a missed special case in the instruction.
> > > >    encoding. I'm working on fixing the assembler.
> >
> > I think this corrects the linker.
> >
> > diff -r 01125acb5565 sys/src/cmd/5l/asm.c
> > --- a/sys/src/cmd/5l/asm.c    Tue Jan 19 19:56:38 2021 -0800
> > +++ b/sys/src/cmd/5l/asm.c    Tue Jan 19 21:56:58 2021 -0800
> > @@ -785,7 +785,7 @@
> >
> >       case 8:         /* sll $c,[R],R -> mov (R<<$c),R */
> >               aclass(&p->from);
> > -             o1 = oprrr(p->as, p->scond);
> > +             o1 = oprrr((instoffset == 0) ? ASLL : p->as, p->scond);
> >               r = p->reg;
> >               if(r == NREG)
> >                       r = p->to.reg;
> >
> > The PDF version of the arm architecture manual
> > I found implied that this is the correct way
> > to do it, and the official arm website is so
> > bad that I can't figure out if it contradicts
> > the PDF from some academic's website.
> >
> > Only lightly tested, not sure if I have any
> > edge cases.
> >
> I like this version more.
>
> diff -r 01125acb5565 sys/src/cmd/5l/asm.c
> --- a/sys/src/cmd/5l/asm.c      Tue Jan 19 19:56:38 2021 -0800
> +++ b/sys/src/cmd/5l/asm.c      Tue Jan 19 22:12:05 2021 -0800
> @@ -789,6 +789,12 @@
>                 r = p->reg;
>                 if(r == NREG)
>                         r = p->to.reg;
> +               /*
> +                * R>>32 is encoded as R>>0, so flip to the
> +                * equivalent R<<0.
> +                */
> +               if(instoffset == 0)
> +                       o1 &= ~(1<<5);
>                 o1 |= r;
>                 o1 |= (instoffset&31) << 7;
>                 o1 |= p->to.reg << 12;
>