<HTML><BODY><div>Hi!</div><div>Thanks for the patch!</div><div>LGTM, except for a few nits below.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16889862742098538107_BODY">From: Mike Pall <mike><br><br>Analyzed by Sergey Kaplun.<br><br>(cherry-picked from commit a01cba9d2d74efc57376822aa43db2d5043af5a4)<br><br>This patch is the follow-up for the previous one. It removes the</div></div></div></div></blockquote><div>Typo: s/the/a</div><div>Typo: s/for the/to the/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>condition for `maxslot` changing in the case when varargs are defined<br>on the trace (i.e. `framedepth` > 0).</div></div></div></div></blockquote><div>Typo: s/i.e./i.e.,/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>Sergey Kaplun:<br>* added the description and the test for the problem<br><br>Part of tarantool/tarantool#8825<br>---<br> src/lj_record.c | 8 ++------<br> .../lj-1024-varg-maxslot.test.lua | 19 ++++++++++++++++++-<br> 2 files changed, 20 insertions(+), 7 deletions(-)<br><br>diff --git a/src/lj_record.c b/src/lj_record.c<br>index 112524d3..02d9db9e 100644<br>--- a/src/lj_record.c<br>+++ b/src/lj_record.c<br>@@ -1775,12 +1775,8 @@ static void rec_varg(jit_State *J, BCReg dst, ptrdiff_t nresults)<br> if (J->framedepth > 0) { /* Simple case: varargs defined on-trace. */<br> ptrdiff_t i;<br> if (nvararg < 0) nvararg = 0;<br>- if (nresults == -1) {<br>- nresults = nvararg;<br>- J->maxslot = dst + (BCReg)nvararg;<br>- } else if (dst + nresults > J->maxslot) {<br>- J->maxslot = dst + (BCReg)nresults;<br>- }<br>+ if (nresults == -1) nresults = nvararg;<br>+ J->maxslot = dst + (BCReg)nresults;<br> for (i = 0; i < nresults; i++)<br> J->base[dst+i] = i < nvararg ? getslot(J, i - nvararg - 1 - LJ_FR2) : TREF_NIL;<br> } else { /* Unknown number of varargs passed to trace. */<br>diff --git a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua<br>index 14270595..f8d74e8a 100644<br>--- a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua<br>+++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua<br>@@ -3,7 +3,7 @@ local test = tap.test('lj-noticket-varg-usedef'):skipcond({<br> ['Test requires JIT enabled'] = not jit.status(),<br> })<br> <br>-test:plan(1)<br>+test:plan(2)<br> <br> jit.opt.start('hotloop=1')<br> <br>@@ -20,4 +20,21 @@ end<br> <br> test:ok(true, 'BC_VARG recording 0th frame depth')<br> <br>+-- Now the same case, but with additional frame, so VARG slots</div></div></div></div></blockquote><div>Typo: s/with/with an/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>+-- are defined on the trace.</div></div></div></div></blockquote><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>+local function bump_varg_frame(...)</div></div></div></div></blockquote><div>Nit: Not sure about the name here, maybe it is better to call it</div><div>just `varg_frame` or something like that. Feel free to ignore, though.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>+ -- BC_VARG 1 1 0. `...` is nil (argument for the script).<br>+ -- luacheck: ignore<br>+ -- XXX: some condition to use several slots on the Lua stack.<br>+ anchor = 1 >= 1, ...<br>+end<br>+<br>+counter = 0<br>+while counter < 3 do<br>+ counter = counter + 1<br>+ bump_varg_frame()<br>+end<br>+<br>+test:ok(true, 'BC_VARG recording with defined on trace VARG slots')<br>+<br> os.exit(test:check() and 0 or 1)<br>--<br>2.34.1</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div><div> </div></div></blockquote></BODY></HTML>