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

[Paulo César Pereira de Andrade]

Em seg., 5 de set. de 2022 às 14:17, Paul Cercueil
<paul@crapouillou.net> escreveu:
>
>
>
> Le lun., sept. 5 2022 at 14:00:41 -0300, Paulo César Pereira de
> Andrade <paulo.cesar.pereira.de.andrade@gmail.com> a écrit :
> > Em sáb., 3 de set. de 2022 às 12:33, Paul Cercueil
> > <paul@crapouillou.net> escreveu:
> >>
> >>  Hi Paulo,
> >>  [...]
> >
> >   Hi Paul,
> >
> >>  >>  About the "call" check, I didn't debug it yet, but it may be
> >>  >> related to
> >>  >>  calling "near" code as well. I had to use the following
> >> temporary
> >>  >>  workaround in my code to avoid crashes:
> >>  >>
> >> https://github.com/pcercuei/lightrec/blob/master/emitter.c#L35-L47
> >>  >
> >>  >   This is strange. check/*.tst is full of forward and backward
> >> jumps.
> >>  > Not just call.tst has them. But call.tst has a few
> >> forward/backward
> >>  > tests of calls and jumps. Unless the can_sign_extend_jump_p()
> >> macro
> >>  > has some bug in the displacement test, and  was never experienced
> >>  > in the tests.
> >>
> >>  What happens is I have a pointer that I keep in the last JIT_V()
> >>  register, and the jit_jmpi() call uses the same register as a
> >> temporary.
> >
> >   Please double check if it is JIT_V13 or jit_(13), it could be some
> > mistake with jit_v_num() telling there are 14 callee save registers.
> > It also happens to be "r14".
>
> This is JIT_V(JIT_V_NUM - 1) which resolves to "r14", that's correct.
>
> >>  Adding a jit_live() right before the jit_jmpi() fixes it, but it
> >> makes
> >>  me wonder why Lightning considered it okay to use this register in
> >> the
> >>  first place.
> >
> >   I would need at least a jit_print() of all the assembly to
> > understand the
> > reason it is apparently considering the register as dead.
> >
> >   Please also make sure to test with assertions enabled. But this
> > should
> > not make much of a difference if jit_live() is a workaround, so, it
> > should
> > be some condition leading it to think the register is dead.
>
> Here's the output of jit_print() followed by the output of
> jit_disassemble():
>
> L0: r14 r10 /* prolog */
>         #name rec_ADDIU
>         movi r27 0x1
>         #name rec_BNE
>         #note /home/paul/dev/lightrec/emitter.c:223
>         ldxi_i r26 r14 0x8
>         stxi_i 0x4 r14 r27
>         subi r10 r10 0x6
>     beqr L2 r26 r27
> L4:
>         #note /home/paul/dev/lightrec/emitter.c:61
>         movi r27 0xbfc06f04
>     jmpi 0x20000d30
> L2: r14 r10
>         #name rec_JAL
>         #note /home/paul/dev/lightrec/emitter.c:61
>         movi r27 0xbfc06f04
>         stxi_i 0x7c r14 r27
>         movi r27 0xbfc0711c
>         subi r10 r10 0x4
>     jmpi 0x20000d30
> L5:
>         ret
> L3: /* epilog */
>
>
>         0x20002354      li      r27,1
> # rec_BNE:/home/paul/dev/lightrec/emitter.c:223
>         0x20002358      lwz     r26,8(r14)
>         0x2000235c      stw     r27,4(r14)
>         0x20002360      addi    r10,r10,-6
>         0x20002364      cmpw    r26,r27
>         0x20002368      beq     0x20002384
> rec_JAL:/home/paul/dev/lightrec/emitter.c:61
> # rec_BNE:/home/paul/dev/lightrec/emitter.c:61
>         0x2000236c      lis     r27,-16448
>         0x20002370      ori     r27,r27,28420
>         0x20002374      lis     r14,8192
>         0x20002378      ori     r14,r14,3376
>         0x2000237c      mtctr   r14
>         0x20002380      bctr
> # rec_JAL:/home/paul/dev/lightrec/emitter.c:61
>         0x20002384      lis     r27,-16448
>         0x20002388      ori     r27,r27,28420
>         0x2000238c      stw     r27,124(r14)
>         0x20002390      lis     r27,-16448
>         0x20002394      ori     r27,r27,28956
>         0x20002398      addi    r10,r10,-4
>         0x2000239c      lis     r15,8192
>         0x200023a0      ori     r15,r15,3376
>         0x200023a4      mtctr   r15
>         0x200023a8      bctr

  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.

Thanks,
Paulo