lightning
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Paul Cercueil
Subject: Re: [PATCH] ppc: Fix 'calli' when floating-point arguments are passed
Date: Fri, 09 Sep 2022 17:52:57 +0100



Le ven., sept. 9 2022 at 12:30:33 -0300, Paulo César Pereira de Andrade <paulo.cesar.pereira.de.andrade@gmail.com> a écrit :
Em sex., 9 de set. de 2022 às 12:08, Paulo César Pereira de Andrade
<paulo.cesar.pereira.de.andrade@gmail.com> escreveu:

 Em sex., 9 de set. de 2022 às 11:42, Paul Cercueil
 <paul@crapouillou.net> escreveu:
 >
 > Hi Paulo,
 >
 > [snip]
 >
 > >>  The very last break happened as it was calling jit_new_state()
 > >> again,
> >> so I suppose the memory address that was being watched was being
 > >> reused
 > >>  and I stopped the debugging session there.
 > >
> > Please just do call jit_print() after finishing split_branches, for
 > > example:
 > >
> > Breakpoint 1, _split_branches (_jit=0x10111270) at lightning.c:2713
 > > 2713        for (node = _jitc->head; node; node = next) {
 > > (gdb) call _jit_print(_jit)
 > > L0: /* prolog */
 > >     #name
 > >     #note 395
 > > ...
 > >
> > and let me know the output. The likely explanation for it not being
 > > set,
> > assuming lightning code is not modified, is that there is some extra
 > > code changing the value of r26 after it is used as a temporary.
 > > Something
> > like in the range from "L2:" to the "stxi_i 0x3e7c r29 r26" below,
 > > there is
> > some instruction that changes r26, and causes it to assume it is dead
 > > in
 > > that range, and usable as a temporary.
 >
> I'll do better, I'll give you an example that reproduces the issue :)
 > See the attached example.
 >
> Your idea that something must be changing r26 in that slice of code
 > pointed me in the right direction.
> I only changed one line in the example: there is now a jit_movi(r26, 0)
 > line 58.
 >
> I assume Lightning detects that r26 is already 0 anyway (because of the > beqi just before) so it removes this instruction, causing havoc at the
 > same time.

Exactly, the instruction is being removed, and the optimizer getting
 confused.
   This should not happen as the optimizations done are supposed to
 be the fool proof ones, but somehow this pattern triggered a bug.
   I will try to understand it and have a patch later today.

  Just in case, this should work as a temporary workaround:

"""
diff --git a/lib/lightning.c b/lib/lightning.c
index d219e6d..c4f2e91 100644
--- a/lib/lightning.c
+++ b/lib/lightning.c
@@ -1702,12 +1702,14 @@ _jit_optimize(jit_state_t *_jit)
            case jit_code_epilog:
                _jitc->function = NULL;
                break;
+#if 0
            case jit_code_beqi:
                redundant_store(node, 1);
                break;
            case jit_code_bnei:
                redundant_store(node, 0);
                break;
+#endif
            default:
 #if JIT_HASH_CONSTS
                if (mask & jit_cc_a0_flt) {
"""

I simply commented the 'redunant_store()' calls as I didn't know if it was OK for the jit_code_beqi/jit_code_bnei cases to fall in the "default" case.

Looks like the workaround works, thanks for the suggestion.


  I will try to work on a patch, to have the live register computation
work as if the movi was still done, or just remove and no longer use
redundant_store, as this movi removal is causing the problem.

"""
        case jit_code_movi:
        if (regno == jit_regno(iter->u.w)) {
            if (iter->flag || iter->v.w != word)
            return;
            del_node(prev, iter);
            iter = prev;
        }
        break;
"""

  The problem is that the instruction is removed after the computation
of the initial state at the start of the block.

 > Cheers,
 > -Paul

 Thanks,
 Paulo


Cheers,
-Paul





reply via email to

[Prev in Thread] Current Thread [Next in Thread]