<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi!<div class=""><br class=""></div><div class="">I reviewed the one at <a href="https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c" class="">https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c</a></div><div class=""><br class=""></div><div class="">Overall is LGTM, I would like to update the 2nd test message to something like</div><div class="">’trace should not appear due to maxirconst limit’</div><div class=""><br class=""></div><div class="">Sergos<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 28 Jan 2022, at 15:35, Sergey Kaplun <<a href="mailto:skaplun@tarantool.org" class="">skaplun@tarantool.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta charset="UTF-8" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Igor,</span><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">On 28.01.22, Igor Munkin wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Sergey,<br class=""><br class="">On 28.01.22, Sergey Kaplun wrote:<br class=""><blockquote type="cite" class="">Igor,<br class=""><br class="">Thanks for the review!<br class=""><br class=""></blockquote><br class=""><snipped><br class=""><br class=""><blockquote type="cite" class=""><blockquote type="cite" class=""><blockquote type="cite" class=""><br class="">Issue: <a href="https://github.com/LuaJIT/LuaJIT/issues/430" class="">https://github.com/LuaJIT/LuaJIT/issues/430</a><br class="">Branch: <a href="https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci" class="">https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci</a><br class="">Tarantool branch: <a href="https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci" class="">https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci</a><br class=""><br class="">src/lj_record.c | 5 ++-<br class="">.../lj-430-maxirconst.test.lua | 43 +++++++++++++++++++<br class="">2 files changed, 46 insertions(+), 2 deletions(-)<br class="">create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua<br class=""><br class=""></blockquote><br class=""><snipped><br class=""><br class=""><blockquote type="cite" class="">diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua<br class="">new file mode 100644<br class="">index 00000000..1829b37d<br class="">--- /dev/null<br class="">+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua<br class="">@@ -0,0 +1,43 @@<br class="">+-- XXX: avoid any other traces compilation due to hotcount<br class="">+-- collisions for predictible results.<br class=""></blockquote></blockquote></blockquote><br class=""><snipped><br class=""><br class=""><blockquote type="cite" class=""><blockquote type="cite" class=""><blockquote type="cite" class="">+jit.off()<br class="">+jit.flush()<br class=""></blockquote><br class="">Minor: I'd rather move this part closer to 'jit.opt.start' to save the<br class="">test structure closer to the other test chunks. Feel free to ignore.<br class=""></blockquote><br class="">I really want to exclude __any__ JIT work here to avoid false-positive<br class="">hotcount (was precendents during writing this test :)). Ignoring.<br class=""></blockquote><br class="">OK, got it.<br class=""><br class=""><blockquote type="cite" class=""><br class=""></blockquote><br class=""><snipped><br class=""><br class=""><blockquote type="cite" class=""><blockquote type="cite" class=""><blockquote type="cite" class="">+test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>'trace number increases')<br class=""></blockquote><br class="">Typo: I doubt we use tabs in Lua sources, but I might be wrong...<br class=""></blockquote><br class="">I suggest to use quarter tabs indent style as we use for C code and Mike<br class="">use in src/jit/*.lua (see bcsave.lua for example).<br class=""><br class="">There was no precedent before, IINM :).<br class=""></blockquote><br class="">See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Fixed, repushed.</span><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class="">+<br class="">+ntrace_old = misc.getmetrics().jit_trace_num<br class="">+jit.on()<br class="">+irconst4()<br class="">+irconst4()<br class="">+jit.off()<br class="">+test:ok(ntrace_old == misc.getmetrics().jit_trace_num,<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>'trace number is the same')<br class=""></blockquote><br class="">Ditto.<br class=""><br class=""><blockquote type="cite" class="">+<br class="">+os.exit(test:check() and 0 or 1)<br class="">--<span class="Apple-converted-space"> </span><br class="">2.34.1<br class=""><br class=""></blockquote><br class="">--<span class="Apple-converted-space"> </span><br class="">Best regards,<br class="">IM<br class=""></blockquote><br class="">--<span class="Apple-converted-space"> </span><br class="">Best regards,<br class="">Sergey Kaplun<br class=""></blockquote><br class="">[1]: <a href="https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua" class="">https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua</a><br class=""><br class="">--<span class="Apple-converted-space"> </span><br class="">Best regards,<br class="">IM<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Best regards,</span><br style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Sergey Kaplun</span></div></blockquote></div><br class=""></div></body></html>