>> > >> > [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.
>>
>> It should go to the default case, because it might need to
>> allocate
>> a constant to be loaded, or if in a function body, need to know
the
>> register is modified, what only matters if it is callee save;
this
>> was
>> a second problem in this code path; if it did not remove the
movi
>> in redundant_store() it should still go to the default case, or,
>> could
>> not detect a callee save register is modified (likely it was
already
>> noted as modified, but maybe in some very special scenario it
>> could not notice that...).
>>
>> > Looks like the workaround works, thanks for the suggestion.
>>
>> The problem was that it basically did add an "implicit" note
note
>> that
>> the register was dead in the range from L2 to the
>> "jit_movi(JIT_V1, 0)",
>> so, it was safe to use it as temporary, but when removing the
>> jit_movi
>> in another optimization pass, it broke the logic.
>>
>> > > 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.
>
> I just pushed a commit with the easiest and I believe safest
> approach to
> keep the optimization and prevent the misuse as a temporary. It
just
> turns the jit_movi into a jit_live.
> This works because the jit_movi() could be seen as the non
existing
> jit_dead()* call. Because it was not used without modification up
to
> that
> removed jit_movi, the start of the block did assume it was dead at
> block entry. It would be too complicated to fully understand the
> condition
> and (not too much) costly to rebuild the initial state. Still, the
> only case
> this approach is bad is if the movi would end up in a dead code
path,
> preventing it from being used as a cheap temporary, or, if run
out of
> temporaries, need a spill/reload.
Sadly it does cause more issues than it solves.
Attached is a piece of code that I generate, with the *workaround*
in
place (not your fix).