<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Test requires jit and it failed on jobs without a JIT</p>
<p>Fixed!<br>
</p>
<div class="moz-cite-prefix">On 7/6/23 14:31, Maxim Kokryashkin
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1688643085.266702017@f729.i.mail.ru">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div>Hi!</div>
<div>Thanks for the fixes!</div>
<div>A few CI jobs are red, please address them.</div>
<div data-signature-widget="container">
<div data-signature-widget="content">
<div>--<br>
Best regards,</div>
<div>Maxim Kokryashkin</div>
</div>
</div>
<div> </div>
<div> </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_16886366040095669465_BODY">
<div class="cl_683199">
<p>Hi, Max!</p>
<p> </p>
<p>Thanks for review! Added more comments to the
test and commit message.</p>
<p>New changes force-pushed to the branch. Please
take a look.</p>
<p> </p>
<p>S.</p>
<div class="moz-cite-prefix_mr_css_attr">On 6/7/23
14:35, Maxim Kokryashkin wrote:</div>
<blockquote type="cite">
<div>Hi, Sergey and Sergey!</div>
<div> </div>
<div>
<div> </div>
</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_mr_css_attr
js-readmsg-msg_mr_css_attr">
<div>
<div
id="style_16860561301802465383_BODY_mr_css_attr">Hi,
Sergey!<br>
Thanks for the patch!<br>
Please, consider my comments
below.<br>
<br>
On 30.05.23, Sergey Bronnikov
wrote:<br>
> From: Sergey Bronnikov <<a
moz-do-not-send="true">sergeyb@tarantool.org</a>><br>
><br>
> Contributed by XmiliaH.<br>
><br>
> (cherry-picked from commit
93a65d3cc263aef2d2feb3d7ff2206aca3bee17e)<br>
><br>
> After emitting bytecode
instruction BC_FNEW fixup is not
required,</div>
</div>
</div>
</div>
</blockquote>
<div>Typo: s/bytecode/the bytecode</div>
</div>
</blockquote>
</blockquote>
<p>Fixed, thanks!</p>
<p> </p>
<blockquote type="cite">
<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>
<div class="js-helper_mr_css_attr
js-readmsg-msg_mr_css_attr">
<div>
<div>> because FuncState will set
a flag PROTO_CHILD that will
trigger emitting<br>
> a pair of instructions
BC_UCLO and BC_RET (see
<src/lj_parse.c:2355>)<br>
> and BC_RET will close all
upvalues from base equal to 0.<br>
<br>
This part describes why replacing
UCLO with FNEW is good enough and<br>
better than just deleting<br>
| case BC_UCLO: return;<br>
But the original problem is that
some of BC_RET are not fixup-ed,
due to<br>
early return, if UCLO is obtained
before, those leads to VM<br>
inconsistency after return from
the function. Please, mention this
too.</div>
</div>
</div>
</div>
</blockquote>
<div>Agree here, it is hard to get what the
patch is about from that description,</div>
<div>without diving into the changes.</div>
</div>
</blockquote>
</blockquote>
<p>Added more details.</p>
<p> </p>
<snipped>
<blockquote type="cite">
<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>
<div class="js-helper_mr_css_attr
js-readmsg-msg_mr_css_attr">
<div>
<div>Also, before the patch I got
the following assertion in JIT:<br>
<br>
| LUA_PATH="src/?.lua;;"
src/luajit -Ohotloop=1 -e '<br>
|<br>
| local function missing_uclo()<br>
| while true do -- luacheck:
ignore<br>
| local f<br>
| if false then break end<br>
| while true do<br>
| if f then<br>
| return f<br>
| end<br>
| f = function()<br>
| return f<br>
| end<br>
| end<br>
| end<br>
| end<br>
| f = missing_uclo()<br>
| print(f())<br>
| f = missing_uclo()<br>
| print(f())<br>
| '<br>
| 3.1002202036551<br>
| luajit:
/home/burii/reviews/luajit/lj-819-missing-uclo/src/lj_record.c:135:
rec_check_slots: Assertion
`((((((tr))>>24) &
IRT_TYPE) - (TRef)(IRT_NUM) <=
(TRef)<br>
| (IRT_INT-IRT_NUM)))' failed.<br>
| Aborted<br>
<br>
I don't sure that we should test
this particular failure too, since
the<br>
origin of the problem is the
incorrect emitted bytecode.<br>
<br>
Thoughts?</div>
</div>
</div>
</div>
</blockquote>
<div>We should not, because it is most
likely caused by the issue</div>
<div>that was fixed in the
LuaJIT/LuaJIT@5c46f477.</div>
</div>
</blockquote>
</blockquote>
<p> </p>
<p>assert in rec_check_slots could be for many
reasons, so I added a testcase for compiler too.</p>
<p> </p>
<blockquote type="cite">
<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>
<div class="js-helper_mr_css_attr
js-readmsg-msg_mr_css_attr">
<div>
<div><br>
> --<br>
> 2.34.1<br>
><br>
<br>
--<br>
Best regards,<br>
Sergey Kaplun</div>
</div>
</div>
</div>
</blockquote>
<div>
<div>--<br>
Best regards,</div>
<div>Maxim Kokryashkin</div>
</div>
<div> </div>
</div>
</blockquote>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<div> </div>
</div>
</blockquote>
</blockquote>
</body>
</html>