Re: [Tarantool-patches] [PATCH luajit 17/19] MIPS64: Fix register allocation in assembly of HREF.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the review!

On 17.08.23, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> 
> Thanks for the patch!
> 
> Test is passed after reverting the patch.
> 
> 
> On 8/9/23 18:36, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Contributed by James Cowgill.
> >
> > (cherry-picked from commit 99cdfbf6a1e8856f64908072ef10443a7eab14f2)
> >
> > The issue is observed for the following merged IRs:
> > |    p64 HREF   0001  "a"            ; or other keys
> > | >  p64 EQ     0002  [0x4002d0c528] ; nilnode
> > Sometimes, when we need to rematerialize a constant during evicting of
> > the register. So, the instruction related to constant rematerialization
> > is placed in the delay branch slot, which suppose to contain the loads
> > of trace exit number to the `$ra` register. The resulting assembly is
> > the following (for example):
> > | beq     ra, r1, 0x400abee9b0  ->exit
> > | lui     r1, 65531   ; delay slot without setting of the `ra`
> > This leading to the assertion failure during trace exit in
> > `lj_trace_exit()`, since a trace number is incorrect.
> >
> > This patch moves the constant register allocations above the main
> > instruction emitting code in `asm_href()`.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> 
> Test is passed after reverting the patch. LuaJIT was built with and 
> without GC64.

The test case is for MIPS, since the changes are only for MIPS too.
But in general it is good practise to test other arches too, for
observing any inconsistencies.

> 
> 
> <snipped>

-- 
Best regards,
Sergey Kaplun