Re: [Tarantool-patches] [PATCH luajit 2/2] Fix maxslots when recording BC_VARG, part 2.

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

Hi, Igor!
Thanks for the review!

On 18.07.23, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! Again, everything is fine with the commit message
> after resolving the comments left by Max, but the black voodoo magic
> condition is still unclear (see my comments to the first patch).
> 
> On 10.07.23, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> > 
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
> > index 14270595..f8d74e8a 100644
> > --- a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
> > +++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
> 
> <snipped>
> 
> > @@ -20,4 +20,21 @@ end
> >  
> >  test:ok(true, 'BC_VARG recording 0th frame depth')
> >  
> > +-- Now the same case, but with additional frame, so VARG slots
> > +-- are defined on the trace.
> > +local function bump_varg_frame(...)
> > +  -- BC_VARG 1 1 0. `...` is nil (argument for the script).
> > +  -- luacheck: ignore
> > +  -- XXX: some condition to use several slots on the Lua stack.
> > +  anchor = 1 >= 1, ...
> 
> Clarification is required here as well as for the first test.

Fixed!

> 
> > +end
> > +
> > +counter = 0
> > +while counter < 3 do
> > +  counter = counter + 1
> > +  bump_varg_frame()
> > +end
> > +
> > +test:ok(true, 'BC_VARG recording with defined on trace VARG slots')
> 
> Minor: I believe it should be "BC_VARG recording with VARG slots defined
> on the trace", but I might be missing something.

Fixed, as you suggested. See the iterative patch below, branch is
force-pushed.

===================================================================
diff --git a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
index ec8afbb5..e33ddba2 100644
--- a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
+++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
@@ -36,8 +36,19 @@ test:ok(true, 'BC_VARG recording 0th frame depth')
 -- are defined on the trace.
 local function varg_frame(...)
   -- BC_VARG 1 1 0. `...` is nil (argument for the script).
+  -- We have the following bytecodes to be recorded:
+  -- 0001  . . KSHORT   0   1
+  -- 0002  . . KSHORT   1   1
+  -- 0003  . . ISLE     0   1
+  -- 0004  . . JMP      0 => 0007
+  -- 0007  . . KPRI     0   2
+  -- 0008  . . VARG     1   1   0
+  --
+  -- 0002 KSHORT bytecode uses the 2nd JIT slot and the 1st Lua
+  -- slot. This Lua slot will be set to nil after 0008 VARG
+  -- bytecode execution, so after VARG recording maxslot should
+  -- point to the 1st JIT slot.
   -- luacheck: ignore
-  -- XXX: some condition to use several slots on the Lua stack.
   anchor = 1 >= 1, ...
 end
 
@@ -47,6 +58,6 @@ while counter < 3 do
   varg_frame()
 end
 
-test:ok(true, 'BC_VARG recording with defined on trace VARG slots')
+test:ok(true, 'BC_VARG recording with VARG slots defined on trace')
 
 os.exit(test:check() and 0 or 1)
===================================================================

> 
> > +
> >  os.exit(test:check() and 0 or 1)
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun