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

[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! The commit message is OK after fixing Max nits,
> but I still have some questions regarding the test.
> 
> 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
> > new file mode 100644
> > index 00000000..14270595
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
> > @@ -0,0 +1,23 @@
> > +local tap = require('tap')
> > +local test = tap.test('lj-noticket-varg-usedef'):skipcond({
> 
> Now you have a ticket number.

Fixed, thanks!

> 
> > +  ['Test requires JIT enabled'] = not jit.status(),
> > +})
> > +
> > +test:plan(1)
> > +
> > +jit.opt.start('hotloop=1')
> > +
> > +local counter = 0
> > +-- luacheck: ignore
> > +local anchor
> > +while counter < 3 do
> > +  counter = counter + 1
> > +  -- BC_VARG 5 1 0. `...` is nil (argument for the script).
> > +  -- luacheck: ignore
> > +  -- XXX: some condition to use several slots on the Lua stack.
> > +  anchor = 1 >= 1, ...
> 
> Well, I have no idea, why this black voodoo magic is required. Comment
> doesn't make it clear either. It would be nice to describe the purpose
> of this in a more verbose way.

Added more verbose description. See the full diff 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 14270595..9a968b0c 100644
--- a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
+++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
@@ -1,5 +1,5 @@
 local tap = require('tap')
-local test = tap.test('lj-noticket-varg-usedef'):skipcond({
+local test = tap.test('lj-1024-varg-usedef'):skipcond({
   ['Test requires JIT enabled'] = not jit.status(),
 })
 
@@ -13,8 +13,20 @@ local anchor
 while counter < 3 do
   counter = counter + 1
   -- BC_VARG 5 1 0. `...` is nil (argument for the script).
+  -- We have the following bytecodes to be recorded:
+  -- 0031  ADDVN    2   2   0  ; 1
+  -- 0032  KSHORT   4   1
+  -- 0033  KSHORT   5   1
+  -- 0034  ISLE     4   5
+  -- 0035  JMP      4 => 0038
+  -- 0038  KPRI     4   2
+  -- 0039  VARG     5   1   0
+  --
+  -- 0033 KSHORT bytecode uses the 6th JIT slot and the 5th Lua
+  -- slot. This Lua slot will be set to nil after 0039 VARG
+  -- bytecode execution, so after VARG recording maxslot should
+  -- point to the 5th JIT slot.
   -- luacheck: ignore
-  -- XXX: some condition to use several slots on the Lua stack.
   anchor = 1 >= 1, ...
 end
 
===================================================================

> 
> > +end
> > +
> > +test:ok(true, 'BC_VARG recording 0th frame depth')
> > +
> > +os.exit(test:check() and 0 or 1)
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun