Re: [PATCH] ppc: Fix 'calli' when floating-point arguments are passed

[Paul Cercueil]

Le mer., sept. 7 2022 at 09:03:04 -0300, Paulo César Pereira de 
Andrade <paulo.cesar.pereira.de.andrade@gmail.com> a écrit :
> Em qua., 7 de set. de 2022 às 07:09, Paul Cercueil
> <paul@crapouillou.net> escreveu:
> >  Hi Paulo,
> >
> >  Le mar., sept. 6 2022 at 09:43:06 -0300, Paulo César Pereira de
> >  Andrade <paulo.cesar.pereira.de.andrade@gmail.com> a écrit :
> >  > Em seg., 5 de set. de 2022 às 17:15, Paul Cercueil
> >  > <paul@crapouillou.net> escreveu:
> >  >>
> >  >>  Hi Paulo,
> >  >>  >
> >  >>  >   This is a not a very common pattern and found a bug in the
> >  >>  > computation of live registers static setup.
> >  >>  >   Please rebuild with the last git push, main difference is 
> > the
> >  >>  > change:
> >  >>  >
> >  >>  > -                       jit_regset_ior(&reglive, &reglive,
> >  >> &regmask);
> >  >>  > +                       jit_regset_ior(&block->reglive, 
> > &reglive,
> >  >>  > &regmask);
> >  >>  >
> >  >>  > The 2 extra return added in the patch is just to avoid extra
> >  >>  > computation
> >  >>  > that is not required, and avoids possible issues setting as 
> > live
> >  >>  > registers
> >  >>  > that are known dead.
> >  >>  >
> >  >>  >   The problem the git commit corrects is that it is setting 
> > only
> >  >> the
> >  >>  > local
> >  >>  > variable and exiting the function, without updating the 
> > initial
> >  >> state
> >  >>  > of the split block after the conditional branch.
> >  >>
> >  >>  I think it did something since now it doesn't crash at the same
> >  >> spot ;)
> >  >>
> >  >>  I get the same thing but with register r10 now. But since r10 
> > is a
> >  >>  caller-saved register I believe this is expected, and I need a
> >  >>  jit_live() there.
> >  >>
> >  >>  If I add a jit_live(_R10) before the jit_jmpi() then everything
> >  >> works.
> >  >>  Thanks!
> >  >>
> >  >>  Note that I use _R10 in my code because I do need a caller-saved
> >  >>  register and none of the JIT_R/JIT_V are. I use it as a counter,
> >  >> and I
> >  >>  need to update it in Lightning-generated functions. If I use a
> >  >>  callee-saved register, Lightning will generate code to 
> > spill/reload
> >  >> the
> >  >>  register and I can't update it within functions.
> >  >
> >  >   To use _R10 you need to cheat a bit, as it is the register for 
> > the
> >  > 8th
> >  > argument.
> >  >   Please also note that with the last patch, now it does what it
> >  > was supposed to do before, that is to assume all callee save 
> > registers
> >  > are live once reaching a jump to an address it cannot determine :(
> >  > And if there isn't a free non callee save register, it will 
> > assert at
> >  > that
> >  > point, as it needs a temporary that cannot be spilled, because it 
> > does
> >  > not know the target, nor can added code to reload the register 
> > used
> >  > temporarily, if it did run out of registers.
> >
> >  Sadly I still see a few problems. See the attached file.
> >
> >  The problem is now that the second bswapr_ui() (after the L2 label)
> >  uses my callee-saved register r25 (aka. JIT_V2) as a temporary, 
> > while
> >  this register is live...
>
>   The issue should be fixed now. It should be due to following blocks
> and not updating r25 as live because it was modified in a previous
> block, so, was not live at the start of a previous block.
>   This again only happens due to jumps to raw addresses, and not
> being able to follow the live state.
>   Please confirm the issue is now corrected with the last patch.

Unfortunately it now starts to use my _R10 register as a temporary... 
see attachment.

It seems to be detected properly as live at label L3, but it is not 
detected as live at label L6, and it should be.

Cheers,
-Paul

lightning_ppc2.txt
Description: Text document