<HTML><BODY><div>Hi, Igor!</div><div>Thanks for the patch!</div><div>LGTM, except for the two nits below.</div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Четверг, 1 декабря 2022, 23:42 +03:00 от Igor Munkin <imun@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16699273741035108938_BODY">JIT engine setup in tarantool-tests/lj-430-maxirconst.test.lua is too<br>tight for the build with LUAJIT_ENABLE_CHECKHOOK option enabled.<br>Originally test limits <maxirconst> value with 3 for the default IRRef<br>constants set: nil, true, false. However, with LUAJIT_ENABLE_CHECKHOOK<br>enabled there are three more auxiliary IRRef constants for the internal<br>purposes. Hence, the test is reimplemented the following way:<br>* The first trace for the empty function is recorded<br>* The number of IRRef constants is obtained via <jit.util.traceinfo><br>* <maxirconst> parameter is set to this value<br>* The second trace for the function with a single local variable,<br> initialized with a constant, fails to record<br><br>As a result there is no need to explicitly respect any configuration.<br><br>Relates to tarantool/tarantool#7762<br><br>Signed-off-by: Igor Munkin <<a href="/compose?To=imun@tarantool.org">imun@tarantool.org</a>><br>---<br> .../lj-430-maxirconst.test.lua | 23 +++++++++++--------<br> 1 file changed, 14 insertions(+), 9 deletions(-)<br><br>diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua<br>index cd3587a8..eaa0cd1b 100644<br>--- a/test/tarantool-tests/lj-430-maxirconst.test.lua<br>+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua<br>@@ -1,8 +1,3 @@<br>--- XXX: avoid any other traces compilation due to hotcount<br>--- collisions for predictable results.<br>-jit.off()<br>-jit.flush()<br>-<br> -- Disabled on *BSD due to #4819.<br> require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')<br> <br>@@ -12,10 +7,6 @@ local traceinfo = require('jit.util').traceinfo<br> local test = tap.test('lj-430-maxirconst')<br> test:plan(2)<br> <br>--- XXX: trace always has at least 3 IR constants: for nil, false<br>--- and true.<br>-jit.opt.start('hotloop=1', 'maxirconst=3')<br>-<br> -- This function has only 3 IR constant.<br> local function irconst3()<br> end<br>@@ -25,6 +16,12 @@ local function irconst4()<br> local _ = 42<br> end<br> <br>+-- XXX: Avoid any other traces compilation due to hotcount<br>+-- collisions for predictable results.<br>+jit.off()<br>+jit.flush()<br>+jit.opt.start('hotloop=1')<br>+<br> assert(not traceinfo(1), 'no traces compiled after flush')<br> jit.on()<br> irconst3()<br>@@ -32,6 +29,14 @@ irconst3()<br> jit.off()<br> test:ok(traceinfo(1), 'new trace created')<br> <br>+-- XXX: Trace 1 always has at least 3 IR constants: for nil, false<br>+-- and true. In case when LUAJIT_ENABLE_CHECKHOOK is set to ON,</div></div></div></div></blockquote><div>Typo: s/In case when/If/</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>+-- three more constants are emitted to the trace.<br>+-- Tight up <maxirconst> and try to record the next trace with one</div></div></div></div></blockquote><div>Typo: s/Tight up/Tighten up/</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>+-- more constant to be emitted.<br>+jit.opt.start(('maxirconst=%d'):format(traceinfo(1).nk))<br>+<br>+assert(not traceinfo(2), 'only one trace is created to this moment')<br> jit.on()<br> irconst4()<br> irconst4()<br>--<br>2.34.0</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></BODY></HTML>