[Tarantool-patches] [PATCH luajit 0/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Tarantool-patches] [PATCH luajit 0/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

The backported commit needs to be checked precisely -- we don't want to
just check that the assertion failures are gone -- some commits [1] may
replace the aforementioned assertions with `return 0` or something else
[2] due to other issues. Hence, we need to be sure that there is no such
IR as "NEWREF.*-0" for these traces anymore. So the first commit
introduces new utility module for tests to parse traces dumped into the
file. There is nothing too fancy -- for now, the returned traces may
only say is there some particular IR or no. Any other features may be
added in the future uses, if we need.

Q: Can we use `jit.dump()` as is, without temporary file?
A: Yes, but no:
   `jit.dump()` may be easily patched to use a custom object as the
   second argument, not only file name. But this module dumps trace
   information not by lines but more iterative. So we can use the
   similar approach as in <luajit-gdb.py> -- concat each line and only
   then dump it. But I don't ready to discuss this opportunity with Mike
   now:). So, for now just use a temporary file and remove it after
   usage.

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-noticket-folding-0
Taranrool PR: https://github.com/tarantool/tarantool/pull/8634
Related issues:
* https://github.com/tarantool/tarantool/issues/8516
* https://github.com/LuaJIT/LuaJIT/issues/981

[1]: https://github.com/LuaJIT/LuaJIT/commit/d4c0c6e17ef7edf1f2893bc807746b80612e63e9
[2]: https://github.com/LuaJIT/LuaJIT/issues/994

Mike Pall (1):
  Fix canonicalization of +-0.0 keys for IR_NEWREF.

Sergey Kaplun (1):
  test: add utility for parsing `jit.dump`

 src/lj_record.c                               |   2 +
 .../tarantool-tests/lj-981-folding-0.test.lua |  57 +++++++
 test/tarantool-tests/utils/jit_parse.lua      | 156 ++++++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 test/tarantool-tests/lj-981-folding-0.test.lua
 create mode 100644 test/tarantool-tests/utils/jit_parse.lua

-- 
2.34.1

[Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches]

This commit adds simple parser for traces to be analyzed in the test.
For now nothing too fancy at all -- just start `jit.dump` to temporary
file and parse and remove this file when dump is stopped. The array
with resulting traces is returned.

For now, just find a single IR by pattern and return ref number and IR,
if exists. More functionality may be added if it will be necessary for
tests.
---
 test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100644 test/tarantool-tests/utils/jit_parse.lua

diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
new file mode 100644
index 00000000..fe8e0e08
--- /dev/null
+++ b/test/tarantool-tests/utils/jit_parse.lua
@@ -0,0 +1,156 @@
+local jdump = require('jit.dump')
+
+local M = {}
+
+local trace_mt = {}
+trace_mt.__index = trace_mt
+
+-- Find and return the first IR ref and value matched the pattern
+-- for the trace.
+trace_mt.has_ir = function(trace, ir_pattern)
+  for i = 1, #trace.ir do
+    local ir = trace.ir[i]
+    if ir:match(ir_pattern) then
+      return i, ir
+    end
+  end
+end
+
+local function trace_new(n)
+  return setmetatable({
+    number = n,
+    parent = nil,
+    parent_exitno = nil,
+    start_loc = nil,
+    bc = {},
+    ir = {},
+    mcode = {},
+    snaps = {},
+  }, trace_mt)
+end
+
+local function parse_bc(trace, line)
+  trace.bc[#trace.bc + 1] = line
+end
+
+local function parse_snap(trace, n_snap, line)
+  assert(trace.snaps[n_snap] == nil)
+  trace[n_snap] = {ref = #trace.ir + 1, slots = line:match('%[(.*)%]$'),}
+end
+
+local function parse_ir(trace, line)
+  local n_snap = line:match('SNAP%s+#(%d+)')
+  if n_snap then
+    parse_snap(trace, n_snap, line)
+    return
+  end
+
+  local current_ins, ir = line:match('^(%d+)%s+(.*)$')
+  current_ins = tonumber(current_ins)
+  -- Insert NOP instruction hidden in IR dump.
+  if current_ins ~= #trace.ir + 1 then
+    trace.ir[#trace.ir + 1] = 'nil NOP'
+  end
+  assert(current_ins == #trace.ir + 1)
+  trace.ir[current_ins] = ir
+end
+
+local function parse_mcode(trace, line)
+  -- Skip loop label.
+  if line == '-> LOOP:' then
+    return
+  end
+  local addr, instruction = line:match('^(%w+)%s+(.*)$')
+  trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
+end
+
+local function parse_trace_hdr(ctx, line, trace_num, status)
+  local traces = ctx.traces
+  if status == 'start' then
+    local trace = trace_new(trace_num)
+    trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
+    -- XXX: Assume, that source line can't contain spaces.
+    -- For example, it's not "(command line)".
+    trace.start_loc = line:match(' ([^%s]+)$')
+    traces[trace_num] = trace
+    ctx.parsing_trace = trace_num
+    ctx.parsing = 'bc'
+  elseif status == 'stop' then
+    assert(ctx.parsing_trace == trace_num)
+    ctx.parsing_trace = nil
+    ctx.parsing = nil
+  elseif status == 'abort' then
+    assert(ctx.parsing_trace == trace_num)
+    ctx.parsing_trace = nil
+    ctx.parsing = nil
+    traces[trace_num] = nil
+  elseif status == 'IR' then
+    ctx.parsing = 'IR'
+  elseif status == 'mcode' then
+    ctx.parsing = 'mcode'
+  else
+    error('Unknown trace status: ' .. status)
+  end
+end
+
+local function parse_line(ctx, line)
+  if line == '' then
+    return
+  end
+
+  if line:match('TRACE flush') then
+    ctx.traces = {}
+    return
+  end
+
+  local trace_num, status = line:match('TRACE (%d+) (%w+)')
+  if trace_num then
+    parse_trace_hdr(ctx, line, tonumber(trace_num), status)
+    return
+  end
+
+  assert(ctx.parsing_trace)
+
+  local trace = ctx.traces[ctx.parsing_trace]
+  if ctx.parsing == 'bc' then
+    parse_bc(trace, line)
+  elseif ctx.parsing == 'IR' then
+    parse_ir(trace, line)
+  elseif ctx.parsing == 'mcode' then
+    parse_mcode(trace, line)
+  end
+end
+
+local JDUMP_FILE
+
+local function parse_jit_dump()
+  local ctx = {traces = {}}
+  for line in io.lines(JDUMP_FILE) do
+    parse_line(ctx, line)
+  end
+  return ctx.traces
+end
+
+M.start = function(flags)
+  assert(JDUMP_FILE == nil, 'jit_parse is already running')
+  -- Always use plain text output.
+  flags = flags .. 'T'
+  JDUMP_FILE = os.tmpname()
+  jdump.start(flags, JDUMP_FILE)
+end
+
+M.finish = function()
+  assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
+  jdump.off()
+  local traces = parse_jit_dump()
+  os.remove(JDUMP_FILE)
+  JDUMP_FILE = nil
+  return traces
+end
+
+-- Turn off compilation for these modules to avoid side effects.
+jit.off(true, true)
+jit.off(jdump.on, true)
+jit.off(jdump.off, true)
+
+return M
-- 
2.34.1

[Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit 96fc114a7a3be3fd2c227d5a0ac53aa50cfb85d1)

This commit is a follow-up for the commit
f067cf638cf8987ab3b6db372d609a5982e458b5 ("Fix narrowing of unary
minus."). Since this commit -0 IR constant is stored as well as +0
constant on the trace. Since IR NEWREF keys don't canonicalizied for -0
opposed of IR HREFK, it may lead to inconsistencies during trace
recording.

In particular, since -0 and 0 are different IR constants, alias analysis
declares that they can't be aliased during folding optimization.
Therefore:
1) For the IR TNEW we have non-nil value to lookup from the table via
   HLOAD, when only nil lookup is expected due to alias analysis.
2) For the IR TDUP we have non-nil value to lookup from the table via
   HLOAD, but the template table has no 0 field initiated as far as -0
   isn't folding to 0 during parsing (see `bcemit_unop()` in
   <src/lj_parse.c>).
These cases lead to the assertion failures in `fwd_ahload()`.

This patch adds the aforementioned canonicalization.

Sergey Bronnikov:
* reported the original issue for the TDUP IR

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#8516
---

Side note: I don't mention the 981 issue by intend -- I don't want to
bother Mike with force pushes:). I suppose Igor should add this line (if
he wants) went this commit will be cherry-picked to our master branch
(a.k.a. tarantool).

 src/lj_record.c                               |  2 +
 .../tarantool-tests/lj-981-folding-0.test.lua | 57 +++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 test/tarantool-tests/lj-981-folding-0.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index 9e2e1d9e..cc44db8d 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -1474,6 +1474,8 @@ TRef lj_record_idx(jit_State *J, RecordIndex *ix)
 	TRef key = ix->key;
 	if (tref_isinteger(key))  /* NEWREF needs a TValue as a key. */
 	  key = emitir(IRTN(IR_CONV), key, IRCONV_NUM_INT);
+	else if (tref_isnumber(key) && tref_isk(key) && tvismzero(&ix->keyv))
+	  key = lj_ir_knum_zero(J);  /* Canonicalize -0.0 to +0.0. */
 	xref = emitir(IRT(IR_NEWREF, IRT_PGC), ix->tab, key);
 	keybarrier = 0;  /* NEWREF already takes care of the key barrier. */
 #ifdef LUAJIT_ENABLE_TABLE_BUMP
diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
new file mode 100644
index 00000000..251da24d
--- /dev/null
+++ b/test/tarantool-tests/lj-981-folding-0.test.lua
@@ -0,0 +1,57 @@
+local tap = require('tap')
+local test = tap.test('lj-981-folding-0'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
+
+-- Test file to demonstrate LuaJIT misbehaviour on load forwarding
+-- for -0 IR constant as table index.
+-- See also, https://github.com/LuaJIT/LuaJIT/issues/981.
+
+local jparse = require('utils.jit_parse')
+
+jit.opt.start('hotloop=1')
+
+test:plan(4)
+
+-- Reset traces.
+jit.flush()
+
+jparse.start('i')
+local result
+local expected = 'result'
+-- TNEW:
+-- -0 isn't folded during parsing, so it will be set with KSHORT,
+-- UNM bytecodes. See <src/lj_parse.c> and bytecode listing
+-- for details.
+-- Because of it, empty table is created via TNEW.
+for _ = 1, 4 do
+  result = ({[-0] = expected})[0]
+end
+
+local traces = jparse.finish()
+
+-- Test that there is no any assertion failure.
+test:ok(result == expected, 'TNEW and -0 folding')
+-- Test that there is no NEWREF -0 IR.
+test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TNEW tab')
+
+-- Reset traces.
+jit.flush()
+
+jparse.start('i')
+-- TDUP:
+-- Now just add a constant field for the table to use TDUP with
+-- template table instead TNEW before -0 is set.
+for _ = 1, 4 do
+  result = ({[-0] = expected, [1] = 1})[0]
+end
+
+traces = jparse.finish()
+
+-- Test that there is no any assertion failure.
+test:ok(result == expected, 'TDUP and -0 folding')
+-- Test that there is no NEWREF -0 IR.
+test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TDUP tab')
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 5555 bytes --]


Hi, Sergey!
Thanks for the patch!
Please consider my comments below.
 
> 
>>This commit adds simple parser for traces to be analyzed in the test.
>Typo: s/adds/adds a/
>>For now nothing too fancy at all -- just start `jit.dump` to temporary
>Typo: s/temporary/a temporary/
>>file and parse and remove this file when dump is stopped. The array
>>with resulting traces is returned.
>>
>>For now, just find a single IR by pattern and return ref number and IR,
>>if exists. More functionality may be added if it will be necessary for
>Typo: s/if exists/if they exist/
>Typo: s/it will be/it is/
>>tests.
>>---
>> test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
>> 1 file changed, 156 insertions(+)
>> create mode 100644 test/tarantool-tests/utils/jit_parse.lua
>>
>>diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
>>new file mode 100644
>>index 00000000..fe8e0e08
>>--- /dev/null
>>+++ b/test/tarantool-tests/utils/jit_parse.lua
>>@@ -0,0 +1,156 @@
>>+local jdump = require('jit.dump')
>>+
>>+local M = {}
>>+
>>+local trace_mt = {}
>>+trace_mt.__index = trace_mt
>>+
>>+-- Find and return the first IR ref and value matched the pattern
>>+-- for the trace.
>>+trace_mt.has_ir = function(trace, ir_pattern)
>>+ for i = 1, #trace.ir do
>>+ local ir = trace.ir[i]
>>+ if ir:match(ir_pattern) then
>>+ return i, ir
>>+ end
>>+ end
>>+end
>>+
>>+local function trace_new(n)
>>+ return setmetatable({
>>+ number = n,
>>+ parent = nil,
>>+ parent_exitno = nil,
>>+ start_loc = nil,
>>+ bc = {},
>>+ ir = {},
>>+ mcode = {},
>>+ snaps = {},
>>+ }, trace_mt)
>>+end
>>+
>>+local function parse_bc(trace, line)
>>+ trace.bc[#trace.bc + 1] = line
>>+end
>>+
>>+local function parse_snap(trace, n_snap, line)
>>+ assert(trace.snaps[n_snap] == nil)
>>+ trace[n_snap] = {ref = #trace.ir + 1, slots = line:match('%[(.*)%]$'),}
>>+end
>>+
>>+local function parse_ir(trace, line)
>>+ local n_snap = line:match('SNAP%s+#(%d+)')
>>+ if n_snap then
>>+ parse_snap(trace, n_snap, line)
>>+ return
>>+ end
>>+
>>+ local current_ins, ir = line:match('^(%d+)%s+(.*)$')
>>+ current_ins = tonumber(current_ins)
>>+ -- Insert NOP instruction hidden in IR dump.
>>+ if current_ins ~= #trace.ir + 1 then
>>+ trace.ir[#trace.ir + 1] = 'nil NOP'
>>+ end
>>+ assert(current_ins == #trace.ir + 1)
>>+ trace.ir[current_ins] = ir
>>+end
>>+
>>+local function parse_mcode(trace, line)
>>+ -- Skip loop label.
>>+ if line == '-> LOOP:' then
>>+ return
>>+ end
>>+ local addr, instruction = line:match('^(%w+)%s+(.*)$')
>>+ trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
>>+end
>>+
>>+local function parse_trace_hdr(ctx, line, trace_num, status)
>>+ local traces = ctx.traces
>>+ if status == 'start' then
>>+ local trace = trace_new(trace_num)
>>+ trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
>>+ -- XXX: Assume, that source line can't contain spaces.
>>+ -- For example, it's not "(command line)".
>>+ trace.start_loc = line:match(' ([^%s]+)$')
>>+ traces[trace_num] = trace
>>+ ctx.parsing_trace = trace_num
>>+ ctx.parsing = 'bc'
>>+ elseif status == 'stop' then
>>+ assert(ctx.parsing_trace == trace_num)
>>+ ctx.parsing_trace = nil
>>+ ctx.parsing = nil
>>+ elseif status == 'abort' then
>>+ assert(ctx.parsing_trace == trace_num)
>>+ ctx.parsing_trace = nil
>>+ ctx.parsing = nil
>>+ traces[trace_num] = nil
>Two branches below are almost identical. Is it possible to avoid
>duplication?
>Side note: Also, I like it much more when switch-like constructions
>in Lua are implemented with tables, because it seems to be cleaner.
>Feel free to ignore.
>>+ elseif status == 'IR' then
>>+ ctx.parsing = 'IR'
>>+ elseif status == 'mcode' then
>>+ ctx.parsing = 'mcode'
>>+ else
>>+ error('Unknown trace status: ' .. status)
>>+ end
>>+end
>>+
>>+local function parse_line(ctx, line)
>>+ if line == '' then
>>+ return
>>+ end
>>+
>>+ if line:match('TRACE flush') then
>>+ ctx.traces = {}
>>+ return
>>+ end
>>+
>>+ local trace_num, status = line:match('TRACE (%d+) (%w+)')
>>+ if trace_num then
>>+ parse_trace_hdr(ctx, line, tonumber(trace_num), status)
>>+ return
>>+ end
>>+
>>+ assert(ctx.parsing_trace)
>>+
>>+ local trace = ctx.traces[ctx.parsing_trace]
>>+ if ctx.parsing == 'bc' then
>>+ parse_bc(trace, line)
>>+ elseif ctx.parsing == 'IR' then
>>+ parse_ir(trace, line)
>>+ elseif ctx.parsing == 'mcode' then
>>+ parse_mcode(trace, line)
>>+ end
>>+end
>>+
>>+local JDUMP_FILE
>>+
>>+local function parse_jit_dump()
>>+ local ctx = {traces = {}}
>>+ for line in io.lines(JDUMP_FILE) do
>I think we should perform error checking for that io operation.
>>+ parse_line(ctx, line)
>>+ end
>>+ return ctx.traces
>>+end
>>+
>>+M.start = function(flags)
>>+ assert(JDUMP_FILE == nil, 'jit_parse is already running')
>>+ -- Always use plain text output.
>>+ flags = flags .. 'T'
>Nit: What if either `A` or `H` is present in the flags? I believe, no
>one is going to do such things, but anyway it is worth considering
>as a thing to check. Feel free to ignore.
>>+ JDUMP_FILE = os.tmpname()
>>+ jdump.start(flags, JDUMP_FILE)
>>+end
>>+
>>+M.finish = function()
>>+ assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
>>+ jdump.off()
>>+ local traces = parse_jit_dump()
>>+ os.remove(JDUMP_FILE)
>>+ JDUMP_FILE = nil
>>+ return traces
>>+end
>>+
>>+-- Turn off compilation for these modules to avoid side effects.
>>+jit.off(true, true)
>>+jit.off(jdump.on, true)
>>+jit.off(jdump.off, true)
>>+
>>+return M
>>--
>>2.34.1
>--
>Best regards,
>Maxim Kokryashkin
> 

[-- Attachment #2: Type: text/html, Size: 7501 bytes --]

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 6239 bytes --]


Hi again!
It seems like I keep forgetting a single nit in every
review today.
 
I am concerned about the `test/tarantool-tests/utils/jit_parse.lua`
path, since we already have the `test/tarantool-tests/utils.lua`
module. I think we should rename this directory to `tools` or
something like that.
--
Best regards,
Maxim Kokryashkin
 
  
>Понедельник, 15 мая 2023, 14:12 +03:00 от Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
> 
>Hi, Sergey!
>Thanks for the patch!
>Please consider my comments below.
> 
>> 
>>>This commit adds simple parser for traces to be analyzed in the test.
>>Typo: s/adds/adds a/
>>>For now nothing too fancy at all -- just start `jit.dump` to temporary
>>Typo: s/temporary/a temporary/
>>>file and parse and remove this file when dump is stopped. The array
>>>with resulting traces is returned.
>>>
>>>For now, just find a single IR by pattern and return ref number and IR,
>>>if exists. More functionality may be added if it will be necessary for
>>Typo: s/if exists/if they exist/
>>Typo: s/it will be/it is/
>>>tests.
>>>---
>>> test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
>>> 1 file changed, 156 insertions(+)
>>> create mode 100644 test/tarantool-tests/utils/jit_parse.lua
>>>
>>>diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
>>>new file mode 100644
>>>index 00000000..fe8e0e08
>>>--- /dev/null
>>>+++ b/test/tarantool-tests/utils/jit_parse.lua
>>>@@ -0,0 +1,156 @@
>>>+local jdump = require('jit.dump')
>>>+
>>>+local M = {}
>>>+
>>>+local trace_mt = {}
>>>+trace_mt.__index = trace_mt
>>>+
>>>+-- Find and return the first IR ref and value matched the pattern
>>>+-- for the trace.
>>>+trace_mt.has_ir = function(trace, ir_pattern)
>>>+ for i = 1, #trace.ir do
>>>+ local ir = trace.ir[i]
>>>+ if ir:match(ir_pattern) then
>>>+ return i, ir
>>>+ end
>>>+ end
>>>+end
>>>+
>>>+local function trace_new(n)
>>>+ return setmetatable({
>>>+ number = n,
>>>+ parent = nil,
>>>+ parent_exitno = nil,
>>>+ start_loc = nil,
>>>+ bc = {},
>>>+ ir = {},
>>>+ mcode = {},
>>>+ snaps = {},
>>>+ }, trace_mt)
>>>+end
>>>+
>>>+local function parse_bc(trace, line)
>>>+ trace.bc[#trace.bc + 1] = line
>>>+end
>>>+
>>>+local function parse_snap(trace, n_snap, line)
>>>+ assert(trace.snaps[n_snap] == nil)
>>>+ trace[n_snap] = {ref = #trace.ir + 1, slots = line:match('%[(.*)%]$'),}
>>>+end
>>>+
>>>+local function parse_ir(trace, line)
>>>+ local n_snap = line:match('SNAP%s+#(%d+)')
>>>+ if n_snap then
>>>+ parse_snap(trace, n_snap, line)
>>>+ return
>>>+ end
>>>+
>>>+ local current_ins, ir = line:match('^(%d+)%s+(.*)$')
>>>+ current_ins = tonumber(current_ins)
>>>+ -- Insert NOP instruction hidden in IR dump.
>>>+ if current_ins ~= #trace.ir + 1 then
>>>+ trace.ir[#trace.ir + 1] = 'nil NOP'
>>>+ end
>>>+ assert(current_ins == #trace.ir + 1)
>>>+ trace.ir[current_ins] = ir
>>>+end
>>>+
>>>+local function parse_mcode(trace, line)
>>>+ -- Skip loop label.
>>>+ if line == '-> LOOP:' then
>>>+ return
>>>+ end
>>>+ local addr, instruction = line:match('^(%w+)%s+(.*)$')
>>>+ trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
>>>+end
>>>+
>>>+local function parse_trace_hdr(ctx, line, trace_num, status)
>>>+ local traces = ctx.traces
>>>+ if status == 'start' then
>>>+ local trace = trace_new(trace_num)
>>>+ trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
>>>+ -- XXX: Assume, that source line can't contain spaces.
>>>+ -- For example, it's not "(command line)".
>>>+ trace.start_loc = line:match(' ([^%s]+)$')
>>>+ traces[trace_num] = trace
>>>+ ctx.parsing_trace = trace_num
>>>+ ctx.parsing = 'bc'
>>>+ elseif status == 'stop' then
>>>+ assert(ctx.parsing_trace == trace_num)
>>>+ ctx.parsing_trace = nil
>>>+ ctx.parsing = nil
>>>+ elseif status == 'abort' then
>>>+ assert(ctx.parsing_trace == trace_num)
>>>+ ctx.parsing_trace = nil
>>>+ ctx.parsing = nil
>>>+ traces[trace_num] = nil
>>Two branches below are almost identical. Is it possible to avoid
>>duplication?
>>Side note: Also, I like it much more when switch-like constructions
>>in Lua are implemented with tables, because it seems to be cleaner.
>>Feel free to ignore.
>>>+ elseif status == 'IR' then
>>>+ ctx.parsing = 'IR'
>>>+ elseif status == 'mcode' then
>>>+ ctx.parsing = 'mcode'
>>>+ else
>>>+ error('Unknown trace status: ' .. status)
>>>+ end
>>>+end
>>>+
>>>+local function parse_line(ctx, line)
>>>+ if line == '' then
>>>+ return
>>>+ end
>>>+
>>>+ if line:match('TRACE flush') then
>>>+ ctx.traces = {}
>>>+ return
>>>+ end
>>>+
>>>+ local trace_num, status = line:match('TRACE (%d+) (%w+)')
>>>+ if trace_num then
>>>+ parse_trace_hdr(ctx, line, tonumber(trace_num), status)
>>>+ return
>>>+ end
>>>+
>>>+ assert(ctx.parsing_trace)
>>>+
>>>+ local trace = ctx.traces[ctx.parsing_trace]
>>>+ if ctx.parsing == 'bc' then
>>>+ parse_bc(trace, line)
>>>+ elseif ctx.parsing == 'IR' then
>>>+ parse_ir(trace, line)
>>>+ elseif ctx.parsing == 'mcode' then
>>>+ parse_mcode(trace, line)
>>>+ end
>>>+end
>>>+
>>>+local JDUMP_FILE
>>>+
>>>+local function parse_jit_dump()
>>>+ local ctx = {traces = {}}
>>>+ for line in io.lines(JDUMP_FILE) do
>>I think we should perform error checking for that io operation.
>>>+ parse_line(ctx, line)
>>>+ end
>>>+ return ctx.traces
>>>+end
>>>+
>>>+M.start = function(flags)
>>>+ assert(JDUMP_FILE == nil, 'jit_parse is already running')
>>>+ -- Always use plain text output.
>>>+ flags = flags .. 'T'
>>Nit: What if either `A` or `H` is present in the flags? I believe, no
>>one is going to do such things, but anyway it is worth considering
>>as a thing to check. Feel free to ignore.
>>>+ JDUMP_FILE = os.tmpname()
>>>+ jdump.start(flags, JDUMP_FILE)
>>>+end
>>>+
>>>+M.finish = function()
>>>+ assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
>>>+ jdump.off()
>>>+ local traces = parse_jit_dump()
>>>+ os.remove(JDUMP_FILE)
>>>+ JDUMP_FILE = nil
>>>+ return traces
>>>+end
>>>+
>>>+-- Turn off compilation for these modules to avoid side effects.
>>>+jit.off(true, true)
>>>+jit.off(jdump.on, true)
>>>+jit.off(jdump.off, true)
>>>+
>>>+return M
>>>--
>>>2.34.1
>>--
>>Best regards,
>>Maxim Kokryashkin
>> 
 

[-- Attachment #2: Type: text/html, Size: 8668 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 4713 bytes --]


Hi, Sergey!
Thanks for the patch!
LGTM, except for a few nits regarding the commit message.
 
> 
>>From: Mike Pall <mike>
>>
>>Reported by Sergey Kaplun.
>>
>>(cherry picked from commit 96fc114a7a3be3fd2c227d5a0ac53aa50cfb85d1)
>>
>>This commit is a follow-up for the commit
>>f067cf638cf8987ab3b6db372d609a5982e458b5 ("Fix narrowing of unary
>>minus."). Since this commit -0 IR constant is stored as well as +0
>Typo: s/as well as/as well as the/
>>constant on the trace. Since IR NEWREF keys don't canonicalizied for -0
>Typo: s/don’t canonicalized/don’t get canonicalized/
>>opposed of IR HREFK, it may lead to inconsistencies during trace
>Typo: s/opposed of/as opposed to/
>>recording.
>>
>>In particular, since -0 and 0 are different IR constants, alias analysis
>>declares that they can't be aliased during folding optimization.
>>Therefore:
>>1) For the IR TNEW we have non-nil value to lookup from the table via
>>   HLOAD, when only nil lookup is expected due to alias analysis.
>>2) For the IR TDUP we have non-nil value to lookup from the table via
>>   HLOAD, but the template table has no 0 field initiated as far as -0
>>   isn't folding to 0 during parsing (see `bcemit_unop()` in
>>   <src/lj_parse.c>).
>>These cases lead to the assertion failures in `fwd_ahload()`.
>Typo: s/the assertion/assertion/
>>
>>This patch adds the aforementioned canonicalization.
>>
>>Sergey Bronnikov:
>>* reported the original issue for the TDUP IR
>>
>>Sergey Kaplun:
>>* added the description and the test for the problem
>>
>>Part of tarantool/tarantool#8516
>>---
>>
>>Side note: I don't mention the 981 issue by intend -- I don't want to
>>bother Mike with force pushes:). I suppose Igor should add this line (if
>>he wants) went this commit will be cherry-picked to our master branch
>>(a.k.a. tarantool).
>>
>> src/lj_record.c | 2 +
>> .../tarantool-tests/lj-981-folding-0.test.lua | 57 +++++++++++++++++++
>> 2 files changed, 59 insertions(+)
>> create mode 100644 test/tarantool-tests/lj-981-folding-0.test.lua
>>
>>diff --git a/src/lj_record.c b/src/lj_record.c
>>index 9e2e1d9e..cc44db8d 100644
>>--- a/src/lj_record.c
>>+++ b/src/lj_record.c
>>@@ -1474,6 +1474,8 @@ TRef lj_record_idx(jit_State *J, RecordIndex *ix)
>>  TRef key = ix->key;
>>  if (tref_isinteger(key)) /* NEWREF needs a TValue as a key. */
>>  key = emitir(IRTN(IR_CONV), key, IRCONV_NUM_INT);
>>+ else if (tref_isnumber(key) && tref_isk(key) && tvismzero(&ix->keyv))
>>+ key = lj_ir_knum_zero(J); /* Canonicalize -0.0 to +0.0. */
>>  xref = emitir(IRT(IR_NEWREF, IRT_PGC), ix->tab, key);
>>  keybarrier = 0; /* NEWREF already takes care of the key barrier. */
>> #ifdef LUAJIT_ENABLE_TABLE_BUMP
>>diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
>>new file mode 100644
>>index 00000000..251da24d
>>--- /dev/null
>>+++ b/test/tarantool-tests/lj-981-folding-0.test.lua
>>@@ -0,0 +1,57 @@
>>+local tap = require('tap')
>>+local test = tap.test('lj-981-folding-0'):skipcond({
>>+ ['Test requires JIT enabled'] = not jit.status(),
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>>+
>>+-- Test file to demonstrate LuaJIT misbehaviour on load forwarding
>>+-- for -0 IR constant as table index.
>>+-- See also,  https://github.com/LuaJIT/LuaJIT/issues/981 .
>>+
>>+local jparse = require('utils.jit_parse')
>>+
>>+jit.opt.start('hotloop=1')
>>+
>>+test:plan(4)
>>+
>>+-- Reset traces.
>>+jit.flush()
>>+
>>+jparse.start('i')
>>+local result
>>+local expected = 'result'
>>+-- TNEW:
>>+-- -0 isn't folded during parsing, so it will be set with KSHORT,
>>+-- UNM bytecodes. See <src/lj_parse.c> and bytecode listing
>>+-- for details.
>>+-- Because of it, empty table is created via TNEW.
>>+for _ = 1, 4 do
>>+ result = ({[-0] = expected})[0]
>>+end
>>+
>>+local traces = jparse.finish()
>>+
>>+-- Test that there is no any assertion failure.
>>+test:ok(result == expected, 'TNEW and -0 folding')
>>+-- Test that there is no NEWREF -0 IR.
>>+test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TNEW tab')
>>+
>>+-- Reset traces.
>>+jit.flush()
>>+
>>+jparse.start('i')
>>+-- TDUP:
>>+-- Now just add a constant field for the table to use TDUP with
>>+-- template table instead TNEW before -0 is set.
>>+for _ = 1, 4 do
>>+ result = ({[-0] = expected, [1] = 1})[0]
>>+end
>>+
>>+traces = jparse.finish()
>>+
>>+-- Test that there is no any assertion failure.
>>+test:ok(result == expected, 'TDUP and -0 folding')
>>+-- Test that there is no NEWREF -0 IR.
>>+test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TDUP tab')
>>+
>>+os.exit(test:check() and 0 or 1)
>>--
>>2.34.1
>--
>Best regards,
>Maxim Kokryashkin

[-- Attachment #2: Type: text/html, Size: 6185 bytes --]

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Bronnikov via Tarantool-patches]

Hello, Sergey

Thanks for the patch! See my three comments inline.

On 5/10/23 15:34, Sergey Kaplun wrote:
> This commit adds simple parser for traces to be analyzed in the test.
> For now nothing too fancy at all -- just start `jit.dump` to temporary
> file and parse and remove this file when dump is stopped. The array
> with resulting traces is returned.
>
> For now, just find a single IR by pattern and return ref number and IR,
> if exists. More functionality may be added if it will be necessary for
> tests.
> ---
>   test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
>   1 file changed, 156 insertions(+)
>   create mode 100644 test/tarantool-tests/utils/jit_parse.lua
>
> diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
> new file mode 100644
> index 00000000..fe8e0e08
> --- /dev/null
> +++ b/test/tarantool-tests/utils/jit_parse.lua

The main purpose of this script is using in tests. It could be dangerous 
to leave it without tests at all,

breakage in it could require time for debugging someday. From other side 
I believe no one wants

writing tests for test infrastructure. I suggest add at least a couple 
of unit tests

that will parse trace in plain text format and check that results are 
equal to expected.

> @@ -0,0 +1,156 @@
> +local jdump = require('jit.dump')
> +
> +local M = {}
> +
> +local trace_mt = {}
> +trace_mt.__index = trace_mt
> +
> +-- Find and return the first IR ref and value matched the pattern
> +-- for the trace.
> +trace_mt.has_ir = function(trace, ir_pattern)
> +  for i = 1, #trace.ir do
> +    local ir = trace.ir[i]
> +    if ir:match(ir_pattern) then
> +      return i, ir
> +    end
> +  end
> +end
> +
> +local function trace_new(n)
> +  return setmetatable({
> +    number = n,
> +    parent = nil,
> +    parent_exitno = nil,
> +    start_loc = nil,
> +    bc = {},
> +    ir = {},
> +    mcode = {},
> +    snaps = {},
> +  }, trace_mt)
> +end
> +
> +local function parse_bc(trace, line)
> +  trace.bc[#trace.bc + 1] = line
> +end
> +
> +local function parse_snap(trace, n_snap, line)
> +  assert(trace.snaps[n_snap] == nil)
> +  trace[n_snap] = {ref = #trace.ir + 1, slots = line:match('%[(.*)%]$'),}
> +end
> +
> +local function parse_ir(trace, line)
> +  local n_snap = line:match('SNAP%s+#(%d+)')
> +  if n_snap then
> +    parse_snap(trace, n_snap, line)
> +    return
> +  end
> +
> +  local current_ins, ir = line:match('^(%d+)%s+(.*)$')
> +  current_ins = tonumber(current_ins)
> +  -- Insert NOP instruction hidden in IR dump.
> +  if current_ins ~= #trace.ir + 1 then
> +    trace.ir[#trace.ir + 1] = 'nil NOP'
> +  end
> +  assert(current_ins == #trace.ir + 1)
> +  trace.ir[current_ins] = ir
> +end
> +
> +local function parse_mcode(trace, line)
> +  -- Skip loop label.
> +  if line == '-> LOOP:' then
> +    return
> +  end
> +  local addr, instruction = line:match('^(%w+)%s+(.*)$')
> +  trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
> +end
> +
> +local function parse_trace_hdr(ctx, line, trace_num, status)
> +  local traces = ctx.traces
> +  if status == 'start' then
> +    local trace = trace_new(trace_num)
> +    trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
> +    -- XXX: Assume, that source line can't contain spaces.
> +    -- For example, it's not "(command line)".
> +    trace.start_loc = line:match(' ([^%s]+)$')
> +    traces[trace_num] = trace
> +    ctx.parsing_trace = trace_num
> +    ctx.parsing = 'bc'
> +  elseif status == 'stop' then
> +    assert(ctx.parsing_trace == trace_num)
> +    ctx.parsing_trace = nil
> +    ctx.parsing = nil
> +  elseif status == 'abort' then
> +    assert(ctx.parsing_trace == trace_num)
> +    ctx.parsing_trace = nil
> +    ctx.parsing = nil
> +    traces[trace_num] = nil
> +  elseif status == 'IR' then
> +    ctx.parsing = 'IR'
> +  elseif status == 'mcode' then
> +    ctx.parsing = 'mcode'
> +  else
> +    error('Unknown trace status: ' .. status)
> +  end
> +end
> +
> +local function parse_line(ctx, line)
> +  if line == '' then
> +    return
> +  end
> +
> +  if line:match('TRACE flush') then
> +    ctx.traces = {}
> +    return
> +  end
> +
> +  local trace_num, status = line:match('TRACE (%d+) (%w+)')
> +  if trace_num then
> +    parse_trace_hdr(ctx, line, tonumber(trace_num), status)
> +    return
> +  end
> +
> +  assert(ctx.parsing_trace)
> +
> +  local trace = ctx.traces[ctx.parsing_trace]
> +  if ctx.parsing == 'bc' then
> +    parse_bc(trace, line)
> +  elseif ctx.parsing == 'IR' then
> +    parse_ir(trace, line)
> +  elseif ctx.parsing == 'mcode' then
> +    parse_mcode(trace, line)
> +  end
> +end
> +
> +local JDUMP_FILE
> +
> +local function parse_jit_dump()
> +  local ctx = {traces = {}}
> +  for line in io.lines(JDUMP_FILE) do
> +    parse_line(ctx, line)
> +  end
> +  return ctx.traces
> +end
> +
> +M.start = function(flags)

Comment with description of function's arguments will be useful here and 
M.finish().

For internal functions too, but it is up to you.

> +  assert(JDUMP_FILE == nil, 'jit_parse is already running')
> +  -- Always use plain text output.
> +  flags = flags .. 'T'
> +  JDUMP_FILE = os.tmpname()
> +  jdump.start(flags, JDUMP_FILE)
> +end
> +
> +M.finish = function()
> +  assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
> +  jdump.off()
> +  local traces = parse_jit_dump()
> +  os.remove(JDUMP_FILE)
> +  JDUMP_FILE = nil
> +  return traces
> +end
> +
> +-- Turn off compilation for these modules to avoid side effects.
> +jit.off(true, true)
> +jit.off(jdump.on, true)
> +jit.off(jdump.off, true)

I would put these three lines on top of file or even disable/enable 
compilation in M.start() and M.finish().

Without enabling compilation back you are mutating environment on using 
this script and

this can be unexpected for those who will use scripts.

> +
> +return M

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Sergey Bronnikov via Tarantool-patches]

Hello, Sergey!

Thanks for the patch! To be honest I'm not proficient enough

for reviewing such patches, but I don't have objections, only a couple 
of minor comments. See below.

To give more confidence with fix I have started fuzzer for luaL_loadbuffer

with applied patch (no crashes for a about 5 hours).


Probably it is worth to mention that PUC Rio Lua has the same behaviour 
when table has indices "-0" and "0":

[0] ~/sources/MRG/tarantool/third_party/luajit$ lua
Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
 > a = {[0] = 1, [2] = 4, [-0] = 7}
 > a[0]
7
 >

I believe it is important because you will say that Lua semantics will 
not broken after your patch.

Sergey


On 5/10/23 15:34, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit 96fc114a7a3be3fd2c227d5a0ac53aa50cfb85d1)
>
> This commit is a follow-up for the commit
> f067cf638cf8987ab3b6db372d609a5982e458b5 ("Fix narrowing of unary
> minus."). Since this commit -0 IR constant is stored as well as +0
> constant on the trace. Since IR NEWREF keys don't canonicalizied for -0
> opposed of IR HREFK, it may lead to inconsistencies during trace
> recording.
>
> In particular, since -0 and 0 are different IR constants, alias analysis
> declares that they can't be aliased during folding optimization.
> Therefore:
> 1) For the IR TNEW we have non-nil value to lookup from the table via
>     HLOAD, when only nil lookup is expected due to alias analysis.
> 2) For the IR TDUP we have non-nil value to lookup from the table via
>     HLOAD, but the template table has no 0 field initiated as far as -0
>     isn't folding to 0 during parsing (see `bcemit_unop()` in
>     <src/lj_parse.c>).
> These cases lead to the assertion failures in `fwd_ahload()`.
>
> This patch adds the aforementioned canonicalization.
>
> Sergey Bronnikov:
> * reported the original issue for the TDUP IR
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#8516
> ---
>
> Side note: I don't mention the 981 issue by intend -- I don't want to
> bother Mike with force pushes:). I suppose Igor should add this line (if
> he wants) went this commit will be cherry-picked to our master branch
> (a.k.a. tarantool).
>
>   src/lj_record.c                               |  2 +
>   .../tarantool-tests/lj-981-folding-0.test.lua | 57 +++++++++++++++++++
>   2 files changed, 59 insertions(+)
>   create mode 100644 test/tarantool-tests/lj-981-folding-0.test.lua
>
> diff --git a/src/lj_record.c b/src/lj_record.c
> index 9e2e1d9e..cc44db8d 100644
> --- a/src/lj_record.c
> +++ b/src/lj_record.c
> @@ -1474,6 +1474,8 @@ TRef lj_record_idx(jit_State *J, RecordIndex *ix)
>   	TRef key = ix->key;
>   	if (tref_isinteger(key))  /* NEWREF needs a TValue as a key. */
>   	  key = emitir(IRTN(IR_CONV), key, IRCONV_NUM_INT);
> +	else if (tref_isnumber(key) && tref_isk(key) && tvismzero(&ix->keyv))
> +	  key = lj_ir_knum_zero(J);  /* Canonicalize -0.0 to +0.0. */
>   	xref = emitir(IRT(IR_NEWREF, IRT_PGC), ix->tab, key);
>   	keybarrier = 0;  /* NEWREF already takes care of the key barrier. */
>   #ifdef LUAJIT_ENABLE_TABLE_BUMP
> diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
> new file mode 100644
> index 00000000..251da24d
> --- /dev/null
> +++ b/test/tarantool-tests/lj-981-folding-0.test.lua
> @@ -0,0 +1,57 @@
> +local tap = require('tap')
> +local test = tap.test('lj-981-folding-0'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
> +})
> +
> +-- Test file to demonstrate LuaJIT misbehaviour on load forwarding
> +-- for -0 IR constant as table index.
> +-- See also, https://github.com/LuaJIT/LuaJIT/issues/981.
> +
> +local jparse = require('utils.jit_parse')
> +
> +jit.opt.start('hotloop=1')

You changed global JIT settings,

it is a good habit to put everything back when test is finished.

> +
> +test:plan(4)
> +
> +-- Reset traces.
> +jit.flush()
> +
> +jparse.start('i')
> +local result
> +local expected = 'result'
> +-- TNEW:
> +-- -0 isn't folded during parsing, so it will be set with KSHORT,
> +-- UNM bytecodes. See <src/lj_parse.c> and bytecode listing
> +-- for details.
> +-- Because of it, empty table is created via TNEW.
> +for _ = 1, 4 do
> +  result = ({[-0] = expected})[0]
> +end
> +
> +local traces = jparse.finish()
> +
> +-- Test that there is no any assertion failure.
> +test:ok(result == expected, 'TNEW and -0 folding')
> +-- Test that there is no NEWREF -0 IR.
> +test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TNEW tab')
> +
> +-- Reset traces.
> +jit.flush()
> +
> +jparse.start('i')
> +-- TDUP:
> +-- Now just add a constant field for the table to use TDUP with
> +-- template table instead TNEW before -0 is set.
> +for _ = 1, 4 do
> +  result = ({[-0] = expected, [1] = 1})[0]
> +end
> +
> +traces = jparse.finish()


To be honest I think that chosen tables in tests are not representative. 
I propose to take this one:


local expected = 1
local result
for _ = 1, 4 do
   result = ({[0] = 1, [-0] = 2})[0]
end

assert(result == 2)

This example clearly demonstrates that element with index "0" was 
superseded by element with index "-0".


> +
> +-- Test that there is no any assertion failure.
> +test:ok(result == expected, 'TDUP and -0 folding')
> +-- Test that there is no NEWREF -0 IR.
> +test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TDUP tab')
> +
> +os.exit(test:check() and 0 or 1)

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Please consider my comments below.

On 16.05.23, Sergey Bronnikov wrote:
> Hello, Sergey!
> 
> Thanks for the patch! To be honest I'm not proficient enough
> 
> for reviewing such patches, but I don't have objections, only a couple 

Never too old to learn!:)
Still, it is good for figuring out how LuaJIT works.

> of minor comments. See below.
> 
> To give more confidence with fix I have started fuzzer for luaL_loadbuffer
> 
> with applied patch (no crashes for a about 5 hours).

Thanks!

> 
> 
> Probably it is worth to mention that PUC Rio Lua has the same behaviour 
> when table has indices "-0" and "0":

I suppose this isn't related to the patch itself:
This is particularity of the parser.
Also, nether patch nor test is about -0 vs 0 parsing as table keys. (*)

> 
> [0] ~/sources/MRG/tarantool/third_party/luajit$ lua
> Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
>  > a = {[0] = 1, [2] = 4, [-0] = 7}
>  > a[0]
> 7
>  >
> 
> I believe it is important because you will say that Lua semantics will 
> not broken after your patch.

Yes, it is good to have such test, I sure. But this isn't related to the
patch and its backporting.
So, ignoring for now.

> 
> Sergey
> 
> 

<snipped>

> > diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
> > new file mode 100644
> > index 00000000..251da24d
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-981-folding-0.test.lua
> > @@ -0,0 +1,57 @@
> > +local tap = require('tap')
> > +local test = tap.test('lj-981-folding-0'):skipcond({
> > +  ['Test requires JIT enabled'] = not jit.status(),
> > +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
> > +})
> > +
> > +-- Test file to demonstrate LuaJIT misbehaviour on load forwarding
> > +-- for -0 IR constant as table index.
> > +-- See also, https://github.com/LuaJIT/LuaJIT/issues/981.
> > +
> > +local jparse = require('utils.jit_parse')
> > +
> > +jit.opt.start('hotloop=1')
> 
> You changed global JIT settings,
> 
> it is a good habit to put everything back when test is finished.

No, actually -- its common approach for bugfix tests:
1) We know, that this file will be executed stand-alone, without
chain-evaluation with other tests, so this setting is applied only for
this particular test.
2) OTOH, for <test/tarantool-tests/misclib-getmetrics-lapi.test.lua> we
use exactly the suggested approach, since there are several JIT settings
to check for different subtests.

> 
> > +
> > +test:plan(4)
> > +
> > +-- Reset traces.
> > +jit.flush()
> > +
> > +jparse.start('i')
> > +local result
> > +local expected = 'result'
> > +-- TNEW:
> > +-- -0 isn't folded during parsing, so it will be set with KSHORT,
> > +-- UNM bytecodes. See <src/lj_parse.c> and bytecode listing
> > +-- for details.
> > +-- Because of it, empty table is created via TNEW.
> > +for _ = 1, 4 do
> > +  result = ({[-0] = expected})[0]
> > +end
> > +
> > +local traces = jparse.finish()
> > +
> > +-- Test that there is no any assertion failure.
> > +test:ok(result == expected, 'TNEW and -0 folding')
> > +-- Test that there is no NEWREF -0 IR.
> > +test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TNEW tab')
> > +
> > +-- Reset traces.
> > +jit.flush()
> > +
> > +jparse.start('i')
> > +-- TDUP:
> > +-- Now just add a constant field for the table to use TDUP with
> > +-- template table instead TNEW before -0 is set.
> > +for _ = 1, 4 do
> > +  result = ({[-0] = expected, [1] = 1})[0]
> > +end
> > +
> > +traces = jparse.finish()
> 
> 
> To be honest I think that chosen tables in tests are not representative. 
> I propose to take this one:
> 
> 
> local expected = 1
> local result
> for _ = 1, 4 do
>    result = ({[0] = 1, [-0] = 2})[0]
> end

I very strong against this approach, this is unrelated to the bug (*)
and confusing for reading.

> 
> assert(result == 2)
> 
> This example clearly demonstrates that element with index "0" was 
> superseded by element with index "-0".

As I said before, this is the good thing to do for checking parser or
Lua semantics correctness, not this particular JIT bug.

> 
> 
> > +
> > +-- Test that there is no any assertion failure.
> > +test:ok(result == expected, 'TDUP and -0 folding')
> > +-- Test that there is no NEWREF -0 IR.
> > +test:ok(not traces[1]:has_ir('NEWREF.*-0'), '-0 is canonized for TDUP tab')
> > +
> > +os.exit(test:check() and 0 or 1)

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the review!
Fixed your comments and force-pushed the branch.

On 15.05.23, Maxim Kokryashkin wrote:
> 
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few nits regarding the commit message.
>  
> > 
> >>From: Mike Pall <mike>
> >>
> >>Reported by Sergey Kaplun.
> >>
> >>(cherry picked from commit 96fc114a7a3be3fd2c227d5a0ac53aa50cfb85d1)
> >>
> >>This commit is a follow-up for the commit
> >>f067cf638cf8987ab3b6db372d609a5982e458b5 ("Fix narrowing of unary
> >>minus."). Since this commit -0 IR constant is stored as well as +0
> >Typo: s/as well as/as well as the/

Fixed.

> >>constant on the trace. Since IR NEWREF keys don't canonicalizied for -0
> >Typo: s/don’t canonicalized/don’t get canonicalized/

Changed to "aren't canonicalizied", thanks!

> >>opposed of IR HREFK, it may lead to inconsistencies during trace
> >Typo: s/opposed of/as opposed to/

Fixed! Thanks!

> >>recording.
> >>
> >>In particular, since -0 and 0 are different IR constants, alias analysis
> >>declares that they can't be aliased during folding optimization.
> >>Therefore:
> >>1) For the IR TNEW we have non-nil value to lookup from the table via
> >>   HLOAD, when only nil lookup is expected due to alias analysis.
> >>2) For the IR TDUP we have non-nil value to lookup from the table via
> >>   HLOAD, but the template table has no 0 field initiated as far as -0
> >>   isn't folding to 0 during parsing (see `bcemit_unop()` in
> >>   <src/lj_parse.c>).
> >>These cases lead to the assertion failures in `fwd_ahload()`.
> >Typo: s/the assertion/assertion/

Fixed!

> >>
> >>This patch adds the aforementioned canonicalization.
> >>
> >>Sergey Bronnikov:
> >>* reported the original issue for the TDUP IR
> >>
> >>Sergey Kaplun:
> >>* added the description and the test for the problem
> >>
> >>Part of tarantool/tarantool#8516
> >>---
> >>
> >>Side note: I don't mention the 981 issue by intend -- I don't want to
> >>bother Mike with force pushes:). I suppose Igor should add this line (if
> >>he wants) went this commit will be cherry-picked to our master branch
> >>(a.k.a. tarantool).
> >>
> >> src/lj_record.c | 2 +
> >> .../tarantool-tests/lj-981-folding-0.test.lua | 57 +++++++++++++++++++
> >> 2 files changed, 59 insertions(+)
> >> create mode 100644 test/tarantool-tests/lj-981-folding-0.test.lua
> >>
> >>diff --git a/src/lj_record.c b/src/lj_record.c
> >>index 9e2e1d9e..cc44db8d 100644
> >>--- a/src/lj_record.c
> >>+++ b/src/lj_record.c

<snipped>

> >>diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
> >>new file mode 100644
> >>index 00000000..251da24d
> >>--- /dev/null
> >>+++ b/test/tarantool-tests/lj-981-folding-0.test.lua

<snipped>

> >>2.34.1
> >--
> >Best regards,
> >Maxim Kokryashkin

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the review!
Please consider my answers below.

On 15.05.23, Maxim Kokryashkin wrote:
> 
> Hi, Sergey!
> Thanks for the patch!
> Please consider my comments below.
>  
> > 
> >>This commit adds simple parser for traces to be analyzed in the test.
> >Typo: s/adds/adds a/

Fixed.

> >>For now nothing too fancy at all -- just start `jit.dump` to temporary
> >Typo: s/temporary/a temporary/

Fixed.

> >>file and parse and remove this file when dump is stopped. The array
> >>with resulting traces is returned.
> >>
> >>For now, just find a single IR by pattern and return ref number and IR,
> >>if exists. More functionality may be added if it will be necessary for
> >Typo: s/if exists/if they exist/
> >Typo: s/it will be/it is/

Fixed, thanks!

> >>tests.
> >>---
> >> test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
> >> 1 file changed, 156 insertions(+)
> >> create mode 100644 test/tarantool-tests/utils/jit_parse.lua
> >>
> >>diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
> >>new file mode 100644
> >>index 00000000..fe8e0e08
> >>--- /dev/null
> >>+++ b/test/tarantool-tests/utils/jit_parse.lua
> >>@@ -0,0 +1,156 @@

<snipped>

> >>+local function parse_trace_hdr(ctx, line, trace_num, status)
> >>+ local traces = ctx.traces
> >>+ if status == 'start' then
> >>+ local trace = trace_new(trace_num)
> >>+ trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
> >>+ -- XXX: Assume, that source line can't contain spaces.
> >>+ -- For example, it's not "(command line)".
> >>+ trace.start_loc = line:match(' ([^%s]+)$')
> >>+ traces[trace_num] = trace
> >>+ ctx.parsing_trace = trace_num
> >>+ ctx.parsing = 'bc'
> >>+ elseif status == 'stop' then
> >>+ assert(ctx.parsing_trace == trace_num)
> >>+ ctx.parsing_trace = nil
> >>+ ctx.parsing = nil
> >>+ elseif status == 'abort' then
> >>+ assert(ctx.parsing_trace == trace_num)
> >>+ ctx.parsing_trace = nil
> >>+ ctx.parsing = nil
> >>+ traces[trace_num] = nil
> >Two branches below are almost identical. Is it possible to avoid
> >duplication?

I suppose, no, especially since we may want to do some more work for
aborted traces (I just don't sure for now).

> >Side note: Also, I like it much more when switch-like constructions
> >in Lua are implemented with tables, because it seems to be cleaner.
> >Feel free to ignore.

Rewritten in this way, see the iterative patch below. Branch is
force-pushed.

===================================================================
diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
index fe8e0e08..7e8f879e 100644
--- a/test/tarantool-tests/utils/jit_parse.lua
+++ b/test/tarantool-tests/utils/jit_parse.lua
@@ -64,9 +64,9 @@ local function parse_mcode(trace, line)
   trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
 end
 
-local function parse_trace_hdr(ctx, line, trace_num, status)
-  local traces = ctx.traces
-  if status == 'start' then
+local header_handlers = {
+  start = function(ctx, line, trace_num)
+    local traces = ctx.traces
     local trace = trace_new(trace_num)
     trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
     -- XXX: Assume, that source line can't contain spaces.
@@ -75,23 +75,27 @@ local function parse_trace_hdr(ctx, line, trace_num, status)
     traces[trace_num] = trace
     ctx.parsing_trace = trace_num
     ctx.parsing = 'bc'
-  elseif status == 'stop' then
+  end,
+  stop = function(ctx, line, trace_num)
+    local traces = ctx.traces
     assert(ctx.parsing_trace == trace_num)
     ctx.parsing_trace = nil
     ctx.parsing = nil
-  elseif status == 'abort' then
+  end,
+  abort = function(ctx, line, trace_num)
+    local traces = ctx.traces
     assert(ctx.parsing_trace == trace_num)
     ctx.parsing_trace = nil
     ctx.parsing = nil
     traces[trace_num] = nil
-  elseif status == 'IR' then
+  end,
+  IR = function(ctx, line, trace_num)
     ctx.parsing = 'IR'
-  elseif status == 'mcode' then
+  end,
+  mcode = function(ctx, line, trace_num)
     ctx.parsing = 'mcode'
-  else
-    error('Unknown trace status: ' .. status)
-  end
-end
+  end,
+}
 
 local function parse_line(ctx, line)
   if line == '' then
@@ -105,7 +109,11 @@ local function parse_line(ctx, line)
 
   local trace_num, status = line:match('TRACE (%d+) (%w+)')
   if trace_num then
-    parse_trace_hdr(ctx, line, tonumber(trace_num), status)
+    if (header_handlers[status]) then
+       header_handlers[status](ctx, line, tonumber(trace_num))
+    else
+      error('Unknown trace status: ' .. status)
+    end
     return
   end
===================================================================

> >>+ elseif status == 'IR' then
> >>+ ctx.parsing = 'IR'
> >>+ elseif status == 'mcode' then
> >>+ ctx.parsing = 'mcode'
> >>+ else
> >>+ error('Unknown trace status: ' .. status)
> >>+ end
> >>+end
> >>+

<snipped>

> >>+
> >>+local JDUMP_FILE
> >>+
> >>+local function parse_jit_dump()
> >>+ local ctx = {traces = {}}
> >>+ for line in io.lines(JDUMP_FILE) do
> >I think we should perform error checking for that io operation.

It is done under the hood:
| $ src/luajit -e 'for _ in io.lines("unexisted") do end '
| src/luajit: (command line):1: bad argument #1 to 'lines' (unexisted: No such file or directory)
| stack traceback:
|         [C]: in function 'lines'
|         (command line):1: in main chunk
|         [C]: at 0x55b73c281064

> >>+ parse_line(ctx, line)
> >>+ end
> >>+ return ctx.traces
> >>+end
> >>+
> >>+M.start = function(flags)
> >>+ assert(JDUMP_FILE == nil, 'jit_parse is already running')
> >>+ -- Always use plain text output.
> >>+ flags = flags .. 'T'
> >Nit: What if either `A` or `H` is present in the flags? I believe, no
> >one is going to do such things, but anyway it is worth considering
> >as a thing to check. Feel free to ignore.

It is rewritten by the last flag (i.e. appended `T`).

> >>+ JDUMP_FILE = os.tmpname()
> >>+ jdump.start(flags, JDUMP_FILE)
> >>+end
> >>+
> >>+M.finish = function()
> >>+ assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
> >>+ jdump.off()
> >>+ local traces = parse_jit_dump()
> >>+ os.remove(JDUMP_FILE)
> >>+ JDUMP_FILE = nil
> >>+ return traces
> >>+end
> >>+
> >>+-- Turn off compilation for these modules to avoid side effects.
> >>+jit.off(true, true)
> >>+jit.off(jdump.on, true)
> >>+jit.off(jdump.off, true)
> >>+
> >>+return M
> >>--
> >>2.34.1
> >--
> >Best regards,
> >Maxim Kokryashkin
> > 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches]

Hi, again! :)

On 15.05.23, Maxim Kokryashkin wrote:
> 
> Hi again!
> It seems like I keep forgetting a single nit in every
> review today.
>  
> I am concerned about the `test/tarantool-tests/utils/jit_parse.lua`
> path, since we already have the `test/tarantool-tests/utils.lua`
> module. I think we should rename this directory to `tools` or
> something like that.

Actually, this was done by intention to accent that this is part of our
utils for testing (it may be considered as a submodule).

> --
> Best regards,
> Maxim Kokryashkin
>  
>   

<snipped>

> >>>2.34.1
> >>--
> >>Best regards,
> >>Maxim Kokryashkin
> >> 
>  

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!

On 16.05.23, Sergey Bronnikov wrote:
> Hello, Sergey
> 
> Thanks for the patch! See my three comments inline.
> 
> On 5/10/23 15:34, Sergey Kaplun wrote:
> > This commit adds simple parser for traces to be analyzed in the test.
> > For now nothing too fancy at all -- just start `jit.dump` to temporary
> > file and parse and remove this file when dump is stopped. The array
> > with resulting traces is returned.
> >
> > For now, just find a single IR by pattern and return ref number and IR,
> > if exists. More functionality may be added if it will be necessary for
> > tests.
> > ---
> >   test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
> >   1 file changed, 156 insertions(+)
> >   create mode 100644 test/tarantool-tests/utils/jit_parse.lua
> >
> > diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
> > new file mode 100644
> > index 00000000..fe8e0e08
> > --- /dev/null
> > +++ b/test/tarantool-tests/utils/jit_parse.lua
> 
> The main purpose of this script is using in tests. It could be dangerous 
> to leave it without tests at all,
> 
> breakage in it could require time for debugging someday. From other side 
> I believe no one wants
> 
> writing tests for test infrastructure. I suggest add at least a couple 
> of unit tests
> 
> that will parse trace in plain text format and check that results are 
> equal to expected.

Added the following unit test:

===================================================================
diff --git a/test/tarantool-tests/unit-jit-parse.test.lua b/test/tarantool-tests/unit-jit-parse.test.lua
new file mode 100644
index 00000000..536135c6
--- /dev/null
+++ b/test/tarantool-tests/unit-jit-parse.test.lua
@@ -0,0 +1,42 @@
+local tap = require('tap')
+local test = tap.test('unit-jit-parse'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+local jparse = require('utils.jit_parse')
+
+local expected_irs = {
+  -- Use IR numbers as keys, for more readable example.
+  -- `%d` is a workaround for GC64 | non-GC64 stack slot number.
+  [1] = 'int SLOAD  #%d+%s+CI',
+  [2] = 'int ADD    0001  %+1',
+  [3] = 'int LE     0002  %+3',
+  [4] = '--- LOOP ------------',
+  [5] = 'int ADD    0002  %+1',
+  [6] = 'int LE     0005  %+3',
+  [7] = 'int PHI    0002  0005',
+}
+local N_TESTS = #expected_irs
+
+jit.opt.start('hotloop=1')
+
+test:plan(N_TESTS)
+
+-- Reset traces.
+jit.flush()
+
+jparse.start('i')
+
+-- Loop to compile:
+for _ = 1, 3 do end
+
+local traces = jparse.finish()
+local loop_trace = traces[1]
+
+for irnum = 1, N_TESTS do
+  local ir_pattern = expected_irs[irnum]
+  local irref = loop_trace:has_ir(ir_pattern)
+  test:is(irref, irnum, 'find IR refernce by pattern: ' .. ir_pattern)
+end
+
+os.exit(test:check() and 0 or 1)
===================================================================

> 
> > @@ -0,0 +1,156 @@

<snipped>

> > +
> > +M.start = function(flags)
> 
> Comment with description of function's arguments will be useful here and 
> M.finish().

Added, the following comments as you suggested:

===================================================================
diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
index 2a9286c9..56267713 100644
--- a/test/tarantool-tests/utils/jit_parse.lua
+++ b/test/tarantool-tests/utils/jit_parse.lua
@@ -139,6 +139,8 @@ local function parse_jit_dump()
   return ctx.traces
 end
 
+-- Start `jit.dump()` utility with the given flags, saving the
+-- output in a temporary file.
 M.start = function(flags)
   assert(JDUMP_FILE == nil, 'jit_parse is already running')
   -- Always use plain text output.
@@ -151,6 +153,9 @@ M.start = function(flags)
   jdump.start(flags, JDUMP_FILE)
 end
 
+-- Stop `jit.dump()` utility parsing the output from the file and
+-- remove this file after.
+-- Returns an array of traces recorded during the run.
 M.finish = function()
   assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
   jdump.off()
===================================================================

> 
> For internal functions too, but it is up to you.

I have no clear vision, how the bytecode or mcode, should be saved for
futher analysis (just save them as is for now). So, I don't leave any
comments near the internal functions.

> 
> > +  assert(JDUMP_FILE == nil, 'jit_parse is already running')
> > +  -- Always use plain text output.
> > +  flags = flags .. 'T'
> > +  JDUMP_FILE = os.tmpname()
> > +  jdump.start(flags, JDUMP_FILE)
> > +end
> > +
> > +M.finish = function()
> > +  assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
> > +  jdump.off()
> > +  local traces = parse_jit_dump()
> > +  os.remove(JDUMP_FILE)
> > +  JDUMP_FILE = nil
> > +  return traces
> > +end
> > +
> > +-- Turn off compilation for these modules to avoid side effects.
> > +jit.off(true, true)
> > +jit.off(jdump.on, true)
> > +jit.off(jdump.off, true)
> 
> I would put these three lines on top of file or even disable/enable 
> compilation in M.start() and M.finish().
> 
> Without enabling compilation back you are mutating environment on using 
> this script and
> 
> this can be unexpected for those who will use scripts.

Yes, this is reasonable, fixed.
But the first line, you mentioned disable JIT compilation for this
particular module, so I just leave it as is.
See the iterative patch below.

===================================================================
diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
index 7e8f879e..2a9286c9 100644
--- a/test/tarantool-tests/utils/jit_parse.lua
+++ b/test/tarantool-tests/utils/jit_parse.lua
@@ -143,6 +143,10 @@ M.start = function(flags)
   assert(JDUMP_FILE == nil, 'jit_parse is already running')
   -- Always use plain text output.
   flags = flags .. 'T'
+  -- Turn off traces compilation for `jit.dump` to avoid side
+  -- effects for the period of the testing.
+  jit.off(jdump.on, true)
+  jit.off(jdump.off, true)
   JDUMP_FILE = os.tmpname()
   jdump.start(flags, JDUMP_FILE)
 end
@@ -150,15 +154,16 @@ end
 M.finish = function()
   assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
   jdump.off()
+  -- Enable traces compilation for `jit.dump` back.
+  jit.on(jdump.on, true)
+  jit.on(jdump.off, true)
   local traces = parse_jit_dump()
   os.remove(JDUMP_FILE)
   JDUMP_FILE = nil
   return traces
 end
 
--- Turn off compilation for these modules to avoid side effects.
+-- Turn off compilation for the module to avoid side effects.
 jit.off(true, true)
-jit.off(jdump.on, true)
-jit.off(jdump.off, true)
 
 return M
===================================================================

> 
> > +
> > +return M

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches]

Hi, again!
Just some followup fixes, for the previous changes.

On 21.05.23, Sergey Kaplun via Tarantool-patches wrote:

<snipped>

> 
> > >>+local function parse_trace_hdr(ctx, line, trace_num, status)
> > >>+ local traces = ctx.traces
> > >>+ if status == 'start' then
> > >>+ local trace = trace_new(trace_num)
> > >>+ trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
> > >>+ -- XXX: Assume, that source line can't contain spaces.
> > >>+ -- For example, it's not "(command line)".
> > >>+ trace.start_loc = line:match(' ([^%s]+)$')
> > >>+ traces[trace_num] = trace
> > >>+ ctx.parsing_trace = trace_num
> > >>+ ctx.parsing = 'bc'
> > >>+ elseif status == 'stop' then
> > >>+ assert(ctx.parsing_trace == trace_num)
> > >>+ ctx.parsing_trace = nil
> > >>+ ctx.parsing = nil
> > >>+ elseif status == 'abort' then
> > >>+ assert(ctx.parsing_trace == trace_num)
> > >>+ ctx.parsing_trace = nil
> > >>+ ctx.parsing = nil
> > >>+ traces[trace_num] = nil
> > >Two branches below are almost identical. Is it possible to avoid
> > >duplication?
> 
> I suppose, no, especially since we may want to do some more work for
> aborted traces (I just don't sure for now).
> 
> > >Side note: Also, I like it much more when switch-like constructions
> > >in Lua are implemented with tables, because it seems to be cleaner.
> > >Feel free to ignore.
> 
> Rewritten in this way, see the iterative patch below. Branch is
> force-pushed.
> 
> ===================================================================
> diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
> index fe8e0e08..7e8f879e 100644
> --- a/test/tarantool-tests/utils/jit_parse.lua
> +++ b/test/tarantool-tests/utils/jit_parse.lua
> @@ -64,9 +64,9 @@ local function parse_mcode(trace, line)
>    trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
>  end
>  
> -local function parse_trace_hdr(ctx, line, trace_num, status)
> -  local traces = ctx.traces
> -  if status == 'start' then
> +local header_handlers = {
> +  start = function(ctx, line, trace_num)
> +    local traces = ctx.traces
>      local trace = trace_new(trace_num)
>      trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
>      -- XXX: Assume, that source line can't contain spaces.
> @@ -75,23 +75,27 @@ local function parse_trace_hdr(ctx, line, trace_num, status)
>      traces[trace_num] = trace
>      ctx.parsing_trace = trace_num
>      ctx.parsing = 'bc'
> -  elseif status == 'stop' then
> +  end,
> +  stop = function(ctx, line, trace_num)
> +    local traces = ctx.traces
>      assert(ctx.parsing_trace == trace_num)
>      ctx.parsing_trace = nil
>      ctx.parsing = nil
> -  elseif status == 'abort' then
> +  end,
> +  abort = function(ctx, line, trace_num)
> +    local traces = ctx.traces
>      assert(ctx.parsing_trace == trace_num)
>      ctx.parsing_trace = nil
>      ctx.parsing = nil
>      traces[trace_num] = nil
> -  elseif status == 'IR' then
> +  end,
> +  IR = function(ctx, line, trace_num)
>      ctx.parsing = 'IR'
> -  elseif status == 'mcode' then
> +  end,
> +  mcode = function(ctx, line, trace_num)
>      ctx.parsing = 'mcode'
> -  else
> -    error('Unknown trace status: ' .. status)
> -  end
> -end
> +  end,
> +}
>  
>  local function parse_line(ctx, line)
>    if line == '' then
> @@ -105,7 +109,11 @@ local function parse_line(ctx, line)
>  
>    local trace_num, status = line:match('TRACE (%d+) (%w+)')
>    if trace_num then
> -    parse_trace_hdr(ctx, line, tonumber(trace_num), status)
> +    if (header_handlers[status]) then
> +       header_handlers[status](ctx, line, tonumber(trace_num))
> +    else
> +      error('Unknown trace status: ' .. status)
> +    end
>      return
>    end
> ===================================================================
> 

Fix luacheck warnings with the following changes:

===================================================================
diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
index 56267713..bd663196 100644
--- a/test/tarantool-tests/utils/jit_parse.lua
+++ b/test/tarantool-tests/utils/jit_parse.lua
@@ -65,7 +65,7 @@ local function parse_mcode(trace, line)
 end
 
 local header_handlers = {
-  start = function(ctx, line, trace_num)
+  start = function(ctx, trace_num, line)
     local traces = ctx.traces
     local trace = trace_new(trace_num)
     trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
@@ -76,23 +76,22 @@ local header_handlers = {
     ctx.parsing_trace = trace_num
     ctx.parsing = 'bc'
   end,
-  stop = function(ctx, line, trace_num)
-    local traces = ctx.traces
+  stop = function(ctx, trace_num)
     assert(ctx.parsing_trace == trace_num)
     ctx.parsing_trace = nil
     ctx.parsing = nil
   end,
-  abort = function(ctx, line, trace_num)
+  abort = function(ctx, trace_num)
     local traces = ctx.traces
     assert(ctx.parsing_trace == trace_num)
     ctx.parsing_trace = nil
     ctx.parsing = nil
     traces[trace_num] = nil
   end,
-  IR = function(ctx, line, trace_num)
+  IR = function(ctx)
     ctx.parsing = 'IR'
   end,
-  mcode = function(ctx, line, trace_num)
+  mcode = function(ctx)
     ctx.parsing = 'mcode'
   end,
 }
@@ -110,7 +109,7 @@ local function parse_line(ctx, line)
   local trace_num, status = line:match('TRACE (%d+) (%w+)')
   if trace_num then
     if (header_handlers[status]) then
-       header_handlers[status](ctx, line, tonumber(trace_num))
+       header_handlers[status](ctx, tonumber(trace_num), line)
     else
       error('Unknown trace status: ' .. status)
     end
===================================================================

Branch is force-pushed.

<snipped>

> > >--
> > >Best regards,
> > >Maxim Kokryashkin
> > > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey!


LGTM now.


On 5/20/23 17:54, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Please consider my comments below.
>
> On 16.05.23, Sergey Bronnikov wrote:
>> Hello, Sergey!
>>
>> Thanks for the patch! To be honest I'm not proficient enough
>>
>> for reviewing such patches, but I don't have objections, only a couple
>

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches]

Hi, again!
I'm brain dead today: totaly forgot about CHECKHOOK, DUALNUM
exotic builds...

I've changed the test to the following:

===================================================================
local tap = require('tap')
local test = tap.test('unit-jit-parse'):skipcond({
  ['Test requires JIT enabled'] = not jit.status(),
  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
})

local jparse = require('utils.jit_parse')

local expected_irs = {
  -- The different exotic builds may add different IR
  -- instructions, so just check some IR-s existence.
  -- `%d` is a workaround for GC64 | non-GC64 stack slot number.
  'int SLOAD  #%d',
  'int ADD    0001  %+1',
}
local N_TESTS = #expected_irs

jit.opt.start('hotloop=1')

test:plan(N_TESTS)

-- Reset traces.
jit.flush()

jparse.start('i')

-- Loop to compile:
for _ = 1, 3 do end

local traces = jparse.finish()
local loop_trace = traces[1]

for irnum = 1, N_TESTS do
  local ir_pattern = expected_irs[irnum]
  local irref = loop_trace:has_ir(ir_pattern)
  test:ok(irref, 'find IR refernce by pattern: ' .. ir_pattern)
end

os.exit(test:check() and 0 or 1)
===================================================================

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 7105 bytes --]


Hi, Sergey!
Thanks for the fixes. LGTM now.
 
--
Best regards,
Maxim Kokryashkin
 
 
> 
>>Hi, Maxim!
>>Thanks for the review!
>>Please consider my answers below.
>>
>>On 15.05.23, Maxim Kokryashkin wrote:
>>>
>>> Hi, Sergey!
>>> Thanks for the patch!
>>> Please consider my comments below.
>>>  
>>> > 
>>> >>This commit adds simple parser for traces to be analyzed in the test.
>>> >Typo: s/adds/adds a/
>>
>>Fixed.
>>
>>> >>For now nothing too fancy at all -- just start `jit.dump` to temporary
>>> >Typo: s/temporary/a temporary/
>>
>>Fixed.
>>
>>> >>file and parse and remove this file when dump is stopped. The array
>>> >>with resulting traces is returned.
>>> >>
>>> >>For now, just find a single IR by pattern and return ref number and IR,
>>> >>if exists. More functionality may be added if it will be necessary for
>>> >Typo: s/if exists/if they exist/
>>> >Typo: s/it will be/it is/
>>
>>Fixed, thanks!
>>
>>> >>tests.
>>> >>---
>>> >> test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++
>>> >> 1 file changed, 156 insertions(+)
>>> >> create mode 100644 test/tarantool-tests/utils/jit_parse.lua
>>> >>
>>> >>diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
>>> >>new file mode 100644
>>> >>index 00000000..fe8e0e08
>>> >>--- /dev/null
>>> >>+++ b/test/tarantool-tests/utils/jit_parse.lua
>>> >>@@ -0,0 +1,156 @@
>>
>><snipped>
>>
>>> >>+local function parse_trace_hdr(ctx, line, trace_num, status)
>>> >>+ local traces = ctx.traces
>>> >>+ if status == 'start' then
>>> >>+ local trace = trace_new(trace_num)
>>> >>+ trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
>>> >>+ -- XXX: Assume, that source line can't contain spaces.
>>> >>+ -- For example, it's not "(command line)".
>>> >>+ trace.start_loc = line:match(' ([^%s]+)$')
>>> >>+ traces[trace_num] = trace
>>> >>+ ctx.parsing_trace = trace_num
>>> >>+ ctx.parsing = 'bc'
>>> >>+ elseif status == 'stop' then
>>> >>+ assert(ctx.parsing_trace == trace_num)
>>> >>+ ctx.parsing_trace = nil
>>> >>+ ctx.parsing = nil
>>> >>+ elseif status == 'abort' then
>>> >>+ assert(ctx.parsing_trace == trace_num)
>>> >>+ ctx.parsing_trace = nil
>>> >>+ ctx.parsing = nil
>>> >>+ traces[trace_num] = nil
>>> >Two branches below are almost identical. Is it possible to avoid
>>> >duplication?
>>
>>I suppose, no, especially since we may want to do some more work for
>>aborted traces (I just don't sure for now).
>>
>>> >Side note: Also, I like it much more when switch-like constructions
>>> >in Lua are implemented with tables, because it seems to be cleaner.
>>> >Feel free to ignore.
>>
>>Rewritten in this way, see the iterative patch below. Branch is
>>force-pushed.
>>
>>===================================================================
>>diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
>>index fe8e0e08..7e8f879e 100644
>>--- a/test/tarantool-tests/utils/jit_parse.lua
>>+++ b/test/tarantool-tests/utils/jit_parse.lua
>>@@ -64,9 +64,9 @@ local function parse_mcode(trace, line)
>>   trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
>> end
>> 
>>-local function parse_trace_hdr(ctx, line, trace_num, status)
>>- local traces = ctx.traces
>>- if status == 'start' then
>>+local header_handlers = {
>>+ start = function(ctx, line, trace_num)
>>+ local traces = ctx.traces
>>     local trace = trace_new(trace_num)
>>     trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
>>     -- XXX: Assume, that source line can't contain spaces.
>>@@ -75,23 +75,27 @@ local function parse_trace_hdr(ctx, line, trace_num, status)
>>     traces[trace_num] = trace
>>     ctx.parsing_trace = trace_num
>>     ctx.parsing = 'bc'
>>- elseif status == 'stop' then
>>+ end,
>>+ stop = function(ctx, line, trace_num)
>>+ local traces = ctx.traces
>>     assert(ctx.parsing_trace == trace_num)
>>     ctx.parsing_trace = nil
>>     ctx.parsing = nil
>>- elseif status == 'abort' then
>>+ end,
>>+ abort = function(ctx, line, trace_num)
>>+ local traces = ctx.traces
>>     assert(ctx.parsing_trace == trace_num)
>>     ctx.parsing_trace = nil
>>     ctx.parsing = nil
>>     traces[trace_num] = nil
>>- elseif status == 'IR' then
>>+ end,
>>+ IR = function(ctx, line, trace_num)
>>     ctx.parsing = 'IR'
>>- elseif status == 'mcode' then
>>+ end,
>>+ mcode = function(ctx, line, trace_num)
>>     ctx.parsing = 'mcode'
>>- else
>>- error('Unknown trace status: ' .. status)
>>- end
>>-end
>>+ end,
>>+}
>> 
>> local function parse_line(ctx, line)
>>   if line == '' then
>>@@ -105,7 +109,11 @@ local function parse_line(ctx, line)
>> 
>>   local trace_num, status = line:match('TRACE (%d+) (%w+)')
>>   if trace_num then
>>- parse_trace_hdr(ctx, line, tonumber(trace_num), status)
>>+ if (header_handlers[status]) then
>>+ header_handlers[status](ctx, line, tonumber(trace_num))
>>+ else
>>+ error('Unknown trace status: ' .. status)
>>+ end
>>     return
>>   end
>>===================================================================
>>
>>> >>+ elseif status == 'IR' then
>>> >>+ ctx.parsing = 'IR'
>>> >>+ elseif status == 'mcode' then
>>> >>+ ctx.parsing = 'mcode'
>>> >>+ else
>>> >>+ error('Unknown trace status: ' .. status)
>>> >>+ end
>>> >>+end
>>> >>+
>>
>><snipped>
>>
>>> >>+
>>> >>+local JDUMP_FILE
>>> >>+
>>> >>+local function parse_jit_dump()
>>> >>+ local ctx = {traces = {}}
>>> >>+ for line in io.lines(JDUMP_FILE) do
>>> >I think we should perform error checking for that io operation.
>>
>>It is done under the hood:
>>| $ src/luajit -e 'for _ in io.lines("unexisted") do end '
>>| src/luajit: (command line):1: bad argument #1 to 'lines' (unexisted: No such file or directory)
>>| stack traceback:
>>| [C]: in function 'lines'
>>| (command line):1: in main chunk
>>| [C]: at 0x55b73c281064
>>
>>> >>+ parse_line(ctx, line)
>>> >>+ end
>>> >>+ return ctx.traces
>>> >>+end
>>> >>+
>>> >>+M.start = function(flags)
>>> >>+ assert(JDUMP_FILE == nil, 'jit_parse is already running')
>>> >>+ -- Always use plain text output.
>>> >>+ flags = flags .. 'T'
>>> >Nit: What if either `A` or `H` is present in the flags? I believe, no
>>> >one is going to do such things, but anyway it is worth considering
>>> >as a thing to check. Feel free to ignore.
>>
>>It is rewritten by the last flag (i.e. appended `T`).
>>
>>> >>+ JDUMP_FILE = os.tmpname()
>>> >>+ jdump.start(flags, JDUMP_FILE)
>>> >>+end
>>> >>+
>>> >>+M.finish = function()
>>> >>+ assert(JDUMP_FILE ~= nil, 'jit_parse is not running')
>>> >>+ jdump.off()
>>> >>+ local traces = parse_jit_dump()
>>> >>+ os.remove(JDUMP_FILE)
>>> >>+ JDUMP_FILE = nil
>>> >>+ return traces
>>> >>+end
>>> >>+
>>> >>+-- Turn off compilation for these modules to avoid side effects.
>>> >>+jit.off(true, true)
>>> >>+jit.off(jdump.on, true)
>>> >>+jit.off(jdump.off, true)
>>> >>+
>>> >>+return M
>>> >>--
>>> >>2.34.1
>>> >--
>>> >Best regards,
>>> >Maxim Kokryashkin
>>> > 
>>
>>--
>>Best regards,
>>Sergey Kaplun
> 

[-- Attachment #2: Type: text/html, Size: 9184 bytes --]

[Tarantool-patches] [PATCH luajit 1/3] test: split utils.lua into several modules

[Igor Munkin via Tarantool-patches]

The next patch introduces a separate JIT-related module with convenient
utils for JIT engine testing. Considering this change it looks vital to
make a structured utils distributed module instead of "all in one" Lua
chunk. As a result the original utils.lua is split into the several
modules per subsystem to be tested (e.g. GC, frontend, profilers, etc.).

Lazy loading of the introduced submodules allows to use this utils in
all test chunks regardless LuaJIT configuration (e.g. with JIT engine
disabled, without FFI support, etc) and do not spoil utils table with
the excess helpers.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---

Sergey, considering the changes you've made in the second patch, I
propose to finally split utils.lua into submodules that loads lazily.
I've pushed my commit on your branch prior to your patchset. If you have
some notes regarding this, please share them. Otherwise, I'll push this
into the LuaJIT long-term branches.


You can also find the trivial changes made within your commits below:

================================================================================

diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
index 64473ba3..d156f53d 100644
--- a/test/tarantool-tests/lj-981-folding-0.test.lua
+++ b/test/tarantool-tests/lj-981-folding-0.test.lua
@@ -8,7 +8,7 @@ local test = tap.test('lj-981-folding-0'):skipcond({
 -- for -0 IR constant as table index.
 -- See also, https://github.com/LuaJIT/LuaJIT/issues/981.
 
-local jparse = require('utils.jit_parse')
+local jparse = require('utils').jit.parse
 
 -- XXX: Avoid any other traces compilation due to hotcount
 -- collisions for predictable results.
diff --git a/test/tarantool-tests/unit-jit-parse.test.lua b/test/tarantool-tests/unit-jit-parse.test.lua
index e9c0bb80..e4445bf4 100644
--- a/test/tarantool-tests/unit-jit-parse.test.lua
+++ b/test/tarantool-tests/unit-jit-parse.test.lua
@@ -4,7 +4,7 @@ local test = tap.test('unit-jit-parse'):skipcond({
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
-local jparse = require('utils.jit_parse')
+local jparse = require('utils').jit.parse
 
 local expected_irs = {
   -- The different exotic builds may add different IR

================================================================================

 test/tarantool-tests/CMakeLists.txt           |   2 +-
 .../bc-jit-unpatching.test.lua                |   5 +-
 .../fix-gc-setupvalue.test.lua                |   4 +-
 .../gh-4427-ffi-sandwich.test.lua             |   2 +-
 .../gh-5813-resolving-of-c-symbols.test.lua   |   2 +-
 ...-missed-carg1-in-bctsetr-fallback.test.lua |   2 +-
 .../lj-351-print-tostring-number.test.lua     |   2 +-
 .../lj-586-debug-non-string-error.test.lua    |   2 +-
 .../lj-flush-on-trace.test.lua                |   2 +-
 .../misclib-getmetrics-lapi.test.lua          |   2 +-
 .../misclib-memprof-lapi.test.lua             |   2 +-
 .../misclib-sysprof-lapi.test.lua             |   2 +-
 test/tarantool-tests/utils.lua                | 125 ------------------
 test/tarantool-tests/utils/exec.lua           |  52 ++++++++
 test/tarantool-tests/utils/frontend.lua       |  25 ++++
 test/tarantool-tests/utils/gc.lua             |  33 +++++
 test/tarantool-tests/utils/init.lua           |   7 +
 test/tarantool-tests/utils/jit/const.lua      |   8 ++
 test/tarantool-tests/utils/jit/init.lua       |   7 +
 test/tarantool-tests/utils/tools.lua          |  15 +++
 20 files changed, 162 insertions(+), 139 deletions(-)
 delete mode 100644 test/tarantool-tests/utils.lua
 create mode 100644 test/tarantool-tests/utils/exec.lua
 create mode 100644 test/tarantool-tests/utils/frontend.lua
 create mode 100644 test/tarantool-tests/utils/gc.lua
 create mode 100644 test/tarantool-tests/utils/init.lua
 create mode 100644 test/tarantool-tests/utils/jit/const.lua
 create mode 100644 test/tarantool-tests/utils/jit/init.lua
 create mode 100644 test/tarantool-tests/utils/tools.lua

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 527905b6..14a98cf2 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -75,7 +75,7 @@ add_subdirectory(lj-flush-on-trace)
 # directory), so LUA_PATH need to be updated.
 make_lua_path(LUA_PATH
   PATHS
-    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
+    ${CMAKE_CURRENT_SOURCE_DIR}/?/init.lua
     ${PROJECT_SOURCE_DIR}/tools/?.lua
     ${LUAJIT_SOURCE_DIR}/?.lua
     ${LUAJIT_BINARY_DIR}/?.lua
diff --git a/test/tarantool-tests/bc-jit-unpatching.test.lua b/test/tarantool-tests/bc-jit-unpatching.test.lua
index 2c3b7c9a..f4f148c9 100644
--- a/test/tarantool-tests/bc-jit-unpatching.test.lua
+++ b/test/tarantool-tests/bc-jit-unpatching.test.lua
@@ -13,12 +13,13 @@ end
 
 local ret1bc = 'RET1%s*1%s*2'
 -- Check that this bytecode still persists.
-assert(utils.hasbc(load(string.dump(f)), ret1bc))
+assert(utils.frontend.hasbc(load(string.dump(f)), ret1bc))
 
 jit.opt.start('hotloop=1', 'hotexit=1')
 -- Compile function to get JLOOP bytecode in recursion.
 f(5)
 
-test:ok(utils.hasbc(load(string.dump(f)), ret1bc), 'bytecode unpatching is OK ')
+test:ok(utils.frontend.hasbc(load(string.dump(f)), ret1bc),
+        'bytecode unpatching is OK')
 
 os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/fix-gc-setupvalue.test.lua b/test/tarantool-tests/fix-gc-setupvalue.test.lua
index c94d5d85..21acc9bf 100644
--- a/test/tarantool-tests/fix-gc-setupvalue.test.lua
+++ b/test/tarantool-tests/fix-gc-setupvalue.test.lua
@@ -1,5 +1,5 @@
 local tap = require('tap')
-local utils = require('utils')
+local gcisblack = require('utils').gc.isblack
 
 local test = tap.test('fix-gc-setupvalue')
 test:plan(1)
@@ -40,7 +40,7 @@ local oldstepmul = collectgarbage('setstepmul', 1)
 
 -- `parent()` function is marked before `child()`, so wait until
 -- it becomes black and proceed with the test.
-while not utils.gcisblack(_G.parent) do
+while not gcisblack(_G.parent) do
   collectgarbage('step')
 end
 
diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index 86544196..677a6085 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -8,7 +8,7 @@ test:plan(2)
 
 -- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
 -- with the given environment, launch options and CLI arguments.
-local script = require('utils').makecmd(arg, {
+local script = require('utils').exec.makecmd(arg, {
   -- XXX: Apple tries their best to "protect their users from
   -- malware". As a result SIP (see the link[1] below) has been
   -- designed and released. Now, Apple developers are so
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
index 9f2c5f85..1209d288 100644
--- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
+++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
@@ -17,7 +17,7 @@ local symtab = require "utils.symtab"
 local testboth = require "resboth"
 local testhash = require "reshash"
 local testgnuhash = require "resgnuhash"
-local profilename = require("utils").profilename
+local profilename = require("utils").tools.profilename
 
 local TMP_BINFILE = profilename("memprofdata.tmp.bin")
 
diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
index 86bbabe3..10db7603 100644
--- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
+++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
@@ -15,7 +15,7 @@ test:plan(2)
 
 -- XXX: We need to make sure the bytecode is present in the chosen
 -- built-in to make sure our test is still valid.
-assert(utils.hasbc(table.move, 'TSETR'))
+assert(utils.frontend.hasbc(table.move, 'TSETR'))
 
 -- `t` table asize equals 1. Just copy its first element (1)
 -- to the field by index 2 > 1, to fallback inside TSETR.
diff --git a/test/tarantool-tests/lj-351-print-tostring-number.test.lua b/test/tarantool-tests/lj-351-print-tostring-number.test.lua
index 72a9ec2b..b7041f2a 100644
--- a/test/tarantool-tests/lj-351-print-tostring-number.test.lua
+++ b/test/tarantool-tests/lj-351-print-tostring-number.test.lua
@@ -3,7 +3,7 @@ local tap = require('tap')
 local test = tap.test('lj-351-print-tostring-number')
 test:plan(8)
 
-local script = require('utils').makecmd(arg)
+local script = require('utils').exec.makecmd(arg)
 
 local cases = {
   {typename = 'nil', value = 'nil'},
diff --git a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
index dcb730a2..c00301a1 100644
--- a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
+++ b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
@@ -21,7 +21,7 @@ test:plan(1)
 -- Debugger prompt.
 local ldb = 'lua_debug> '
 local magic = 42
-local luabin = utils.luacmd(arg)
+local luabin = utils.exec.luacmd(arg)
 local stderr = {
   -- XXX: The first debugger prompt printed at the start.
   ldb,
diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
index 46db4d2a..fe740087 100644
--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
@@ -8,7 +8,7 @@ test:plan(2)
 
 -- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
 -- with the given environment, launch options and CLI arguments.
-local script = require('utils').makecmd(arg, {
+local script = require('utils').exec.makecmd(arg, {
   -- XXX: Apple tries their best to "protect their users from
   -- malware". As a result SIP (see the link[1] below) has been
   -- designed and released. Now, Apple developers are so
diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
index 881e717b..0ee71499 100644
--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
@@ -10,7 +10,7 @@ local test = tap.test("lib-misc-getmetrics"):skipcond({
 
 test:plan(10)
 
-local MAXNINS = require('utils').const.maxnins
+local MAXNINS = require('utils').jit.const.maxnins
 local jit_opt_default = {
     3, -- level
     "hotloop=56",
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 4e413c88..eae20893 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -26,7 +26,7 @@ local bufread = require "utils.bufread"
 local memprof = require "memprof.parse"
 local process = require "memprof.process"
 local symtab = require "utils.symtab"
-local profilename = require("utils").profilename
+local profilename = require("utils").tools.profilename
 
 local TMP_BINFILE = profilename("memprofdata.tmp.bin")
 local BAD_PATH = profilename("memprofdata/tmp.bin")
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index 96eaaab6..2f0635db 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -15,7 +15,7 @@ pcall(jit.flush)
 local bufread = require("utils.bufread")
 local symtab = require("utils.symtab")
 local sysprof = require("sysprof.parse")
-local profilename = require("utils").profilename
+local profilename = require("utils").tools.profilename
 
 local TMP_BINFILE = profilename("sysprofdata.tmp.bin")
 local BAD_PATH = profilename("sysprofdata/tmp.bin")
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
deleted file mode 100644
index 8c1538d6..00000000
--- a/test/tarantool-tests/utils.lua
+++ /dev/null
@@ -1,125 +0,0 @@
-local M = {}
-
-local ffi = require('ffi')
-local bc = require('jit.bc')
-local bit = require('bit')
-
-local LJ_GC_BLACK = 0x04
-local LJ_STR_HASHLEN = 8
-local GCref = ffi.abi('gc64') and 'uint64_t' or 'uint32_t'
-
-ffi.cdef([[
-  typedef struct {
-]]..GCref..[[ nextgc;
-    uint8_t   marked;
-    uint8_t   gct;
-    /* Need this fields for correct alignment and sizeof. */
-    uint8_t   misc1;
-    uint8_t   misc2;
-  } GCHeader;
-]])
-
-function M.gcisblack(obj)
-  local objtype = type(obj)
-  local address = objtype == 'string'
-    -- XXX: get strdata first and go back to GCHeader.
-    and ffi.cast('char *', obj) - (ffi.sizeof('GCHeader') + LJ_STR_HASHLEN)
-    -- XXX: FFI ABI forbids to cast functions objects
-    -- to non-functional pointers, but we can get their address
-    -- via tostring.
-    or tonumber((tostring(obj):gsub(objtype .. ': ', '')))
-  local marked = ffi.cast('GCHeader *', address).marked
-  return bit.band(marked, LJ_GC_BLACK) == LJ_GC_BLACK
-end
-
-function M.luacmd(args)
-  -- arg[-1] is guaranteed to be not nil.
-  local idx = -2
-  while args[idx] do
-    assert(type(args[idx]) == 'string', 'Command part have to be a string')
-    idx = idx - 1
-  end
-  -- return the full command with flags.
-  return table.concat(args, ' ', idx + 1, -1)
-end
-
-local function makeenv(tabenv)
-  if tabenv == nil then return '' end
-  local flatenv = {}
-  for var, value in pairs(tabenv) do
-    table.insert(flatenv, ('%s=%s'):format(var, value))
-  end
-  return table.concat(flatenv, ' ')
-end
-
--- <makecmd> creates a command that runs %testname%/script.lua by
--- <LUAJIT_TEST_BINARY> with the given environment, launch options
--- and CLI arguments. The function yields an object (i.e. table)
--- with the aforementioned parameters. To launch the command just
--- call the object.
-function M.makecmd(arg, opts)
-  return setmetatable({
-    LUABIN = M.luacmd(arg),
-    SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
-    ENV = opts and makeenv(opts.env) or '',
-    REDIRECT = opts and opts.redirect or '',
-  }, {
-    __call = function(self, ...)
-      -- This line just makes the command for <io.popen> by the
-      -- following steps:
-      -- 1. Replace the placeholders with the corresponding values
-      --    given to the command constructor (e.g. script, env).
-      -- 2. Join all CLI arguments given to the __call metamethod.
-      -- 3. Concatenate the results of step 1 and step 2 to obtain
-      --    the resulting command.
-      local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
-                  .. (' %s'):rep(select('#', ...)):format(...)
-      -- Trim both leading and trailing whitespace from the output
-      -- produced by the child process.
-      return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
-    end
-  })
-end
-
-function M.hasbc(f, bytecode)
-  assert(type(f) == 'function', 'argument #1 should be a function')
-  assert(type(bytecode) == 'string', 'argument #2 should be a string')
-  local function empty() end
-  local hasbc = false
-  -- Check the bytecode entry line by line.
-  local out = {
-    write = function(out, line)
-      if line:match(bytecode) then
-        hasbc = true
-        out.write = empty
-      end
-    end,
-    flush = empty,
-    close = empty,
-  }
-  bc.dump(f, out)
-  return hasbc
-end
-
-function M.profilename(name)
-  local vardir = os.getenv('LUAJIT_TEST_VARDIR')
-  -- Replace pattern will change directory name of the generated
-  -- profile to LUAJIT_TEST_VARDIR if it is set in the process
-  -- environment. Otherwise, the original dirname is left intact.
-  -- As a basename for this profile the test name is concatenated
-  -- with the name given as an argument.
-  local replacepattern = ('%s/%s-%s'):format(vardir or '%1', '%2', name)
-  -- XXX: return only the resulting string.
-  return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern))
-end
-
-M.const = {
-  -- XXX: Max nins is limited by max IRRef, that equals to
-  -- REF_DROP - REF_BIAS. Unfortunately, these constants are not
-  -- provided to Lua space, so we ought to make some math:
-  -- * REF_DROP = 0xffff
-  -- * REF_BIAS = 0x8000
-  maxnins = 0xffff - 0x8000,
-}
-
-return M
diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
new file mode 100644
index 00000000..a56ca2dc
--- /dev/null
+++ b/test/tarantool-tests/utils/exec.lua
@@ -0,0 +1,52 @@
+local M = {}
+
+function M.luacmd(args)
+  -- arg[-1] is guaranteed to be not nil.
+  local idx = -2
+  while args[idx] do
+    assert(type(args[idx]) == 'string', 'Command part have to be a string')
+    idx = idx - 1
+  end
+  -- return the full command with flags.
+  return table.concat(args, ' ', idx + 1, -1)
+end
+
+local function makeenv(tabenv)
+  if tabenv == nil then return '' end
+  local flatenv = {}
+  for var, value in pairs(tabenv) do
+    table.insert(flatenv, ('%s=%s'):format(var, value))
+  end
+  return table.concat(flatenv, ' ')
+end
+
+-- <makecmd> creates a command that runs %testname%/script.lua by
+-- <LUAJIT_TEST_BINARY> with the given environment, launch options
+-- and CLI arguments. The function yields an object (i.e. table)
+-- with the aforementioned parameters. To launch the command just
+-- call the object.
+function M.makecmd(arg, opts)
+  return setmetatable({
+    LUABIN = M.luacmd(arg),
+    SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
+    ENV = opts and makeenv(opts.env) or '',
+    REDIRECT = opts and opts.redirect or '',
+  }, {
+    __call = function(self, ...)
+      -- This line just makes the command for <io.popen> by the
+      -- following steps:
+      -- 1. Replace the placeholders with the corresponding values
+      --    given to the command constructor (e.g. script, env).
+      -- 2. Join all CLI arguments given to the __call metamethod.
+      -- 3. Concatenate the results of step 1 and step 2 to obtain
+      --    the resulting command.
+      local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
+                  .. (' %s'):rep(select('#', ...)):format(...)
+      -- Trim both leading and trailing whitespace from the output
+      -- produced by the child process.
+      return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
+    end
+  })
+end
+
+return M
diff --git a/test/tarantool-tests/utils/frontend.lua b/test/tarantool-tests/utils/frontend.lua
new file mode 100644
index 00000000..2afebbb2
--- /dev/null
+++ b/test/tarantool-tests/utils/frontend.lua
@@ -0,0 +1,25 @@
+local M = {}
+
+local bc = require('jit.bc')
+
+function M.hasbc(f, bytecode)
+  assert(type(f) == 'function', 'argument #1 should be a function')
+  assert(type(bytecode) == 'string', 'argument #2 should be a string')
+  local function empty() end
+  local hasbc = false
+  -- Check the bytecode entry line by line.
+  local out = {
+    write = function(out, line)
+      if line:match(bytecode) then
+        hasbc = true
+        out.write = empty
+      end
+    end,
+    flush = empty,
+    close = empty,
+  }
+  bc.dump(f, out)
+  return hasbc
+end
+
+return M
diff --git a/test/tarantool-tests/utils/gc.lua b/test/tarantool-tests/utils/gc.lua
new file mode 100644
index 00000000..22094f51
--- /dev/null
+++ b/test/tarantool-tests/utils/gc.lua
@@ -0,0 +1,33 @@
+local M = {}
+
+local ffi = require('ffi')
+
+local LJ_GC_BLACK = 0x04
+local LJ_STR_HASHLEN = 8
+local GCref = ffi.abi('gc64') and 'uint64_t' or 'uint32_t'
+
+ffi.cdef([[
+  typedef struct {
+]]..GCref..[[ nextgc;
+    uint8_t   marked;
+    uint8_t   gct;
+    /* Need this fields for correct alignment and sizeof. */
+    uint8_t   misc1;
+    uint8_t   misc2;
+  } GCHeader;
+]])
+
+function M.isblack(obj)
+  local objtype = type(obj)
+  local address = objtype == 'string'
+    -- XXX: get strdata first and go back to GCHeader.
+    and ffi.cast('char *', obj) - (ffi.sizeof('GCHeader') + LJ_STR_HASHLEN)
+    -- XXX: FFI ABI forbids to cast functions objects
+    -- to non-functional pointers, but we can get their address
+    -- via tostring.
+    or tonumber((tostring(obj):gsub(objtype .. ': ', '')))
+  local marked = ffi.cast('GCHeader *', address).marked
+  return bit.band(marked, LJ_GC_BLACK) == LJ_GC_BLACK
+end
+
+return M
diff --git a/test/tarantool-tests/utils/init.lua b/test/tarantool-tests/utils/init.lua
new file mode 100644
index 00000000..ecf55ee7
--- /dev/null
+++ b/test/tarantool-tests/utils/init.lua
@@ -0,0 +1,7 @@
+return setmetatable({}, {
+  __index = function(self, k)
+    assert(type(k) == 'string', "The name of `utils' submodule is required")
+    rawset(self, k, require(('utils.%s'):format(k)))
+    return rawget(self, k)
+  end,
+})
diff --git a/test/tarantool-tests/utils/jit/const.lua b/test/tarantool-tests/utils/jit/const.lua
new file mode 100644
index 00000000..13e58830
--- /dev/null
+++ b/test/tarantool-tests/utils/jit/const.lua
@@ -0,0 +1,8 @@
+return {
+  -- XXX: Max nins is limited by max IRRef, that equals to
+  -- REF_DROP - REF_BIAS. Unfortunately, these constants are not
+  -- provided to Lua space, so we ought to make some math:
+  -- * REF_DROP = 0xffff
+  -- * REF_BIAS = 0x8000
+  maxnins = 0xffff - 0x8000,
+}
diff --git a/test/tarantool-tests/utils/jit/init.lua b/test/tarantool-tests/utils/jit/init.lua
new file mode 100644
index 00000000..56662214
--- /dev/null
+++ b/test/tarantool-tests/utils/jit/init.lua
@@ -0,0 +1,7 @@
+return setmetatable({}, {
+  __index = function(self, k)
+    assert(type(k) == 'string', "The name of `utils.jit' submodule is required")
+    rawset(self, k, require(('utils.jit.%s'):format(k)))
+    return rawget(self, k)
+  end,
+})
diff --git a/test/tarantool-tests/utils/tools.lua b/test/tarantool-tests/utils/tools.lua
new file mode 100644
index 00000000..f35c6922
--- /dev/null
+++ b/test/tarantool-tests/utils/tools.lua
@@ -0,0 +1,15 @@
+local M = {}
+
+function M.profilename(name)
+  local vardir = os.getenv('LUAJIT_TEST_VARDIR')
+  -- Replace pattern will change directory name of the generated
+  -- profile to LUAJIT_TEST_VARDIR if it is set in the process
+  -- environment. Otherwise, the original dirname is left intact.
+  -- As a basename for this profile the test name is concatenated
+  -- with the name given as an argument.
+  local replacepattern = ('%s/%s-%s'):format(vardir or '%1', '%2', name)
+  -- XXX: return only the resulting string.
+  return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern))
+end
+
+return M
-- 
2.30.2

Re: [Tarantool-patches] [PATCH luajit 1/3] test: split utils.lua into several modules

[Igor Munkin via Tarantool-patches]

A tiny fixup is coming...

On 27.06.23, Igor Munkin wrote:
> The next patch introduces a separate JIT-related module with convenient
> utils for JIT engine testing. Considering this change it looks vital to
> make a structured utils distributed module instead of "all in one" Lua
> chunk. As a result the original utils.lua is split into the several
> modules per subsystem to be tested (e.g. GC, frontend, profilers, etc.).
> 
> Lazy loading of the introduced submodules allows to use this utils in
> all test chunks regardless LuaJIT configuration (e.g. with JIT engine
> disabled, without FFI support, etc) and do not spoil utils table with
> the excess helpers.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

<snipped>

> 
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 527905b6..14a98cf2 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -75,7 +75,7 @@ add_subdirectory(lj-flush-on-trace)
>  # directory), so LUA_PATH need to be updated.
>  make_lua_path(LUA_PATH
>    PATHS
> -    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua

Oops, this line was removed by mistake: in this case tap.lua can't be
found. Removed this line back:

================================================================================

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 14a98cf2..b1211971 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -75,6 +75,7 @@ add_subdirectory(lj-flush-on-trace)
 # directory), so LUA_PATH need to be updated.
 make_lua_path(LUA_PATH
   PATHS
+    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
     ${CMAKE_CURRENT_SOURCE_DIR}/?/init.lua
     ${PROJECT_SOURCE_DIR}/tools/?.lua
     ${LUAJIT_SOURCE_DIR}/?.lua

================================================================================

> +    ${CMAKE_CURRENT_SOURCE_DIR}/?/init.lua
>      ${PROJECT_SOURCE_DIR}/tools/?.lua
>      ${LUAJIT_SOURCE_DIR}/?.lua
>      ${LUAJIT_BINARY_DIR}/?.lua

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 1/3] test: split utils.lua into several modules

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
Yes, splitting of the testing utils sounds like a good idea, indeed!
With these changes all code becomes more organized.

The patch itself LGTM, except a few nits regarding the commit message.

On 27.06.23, Igor Munkin wrote:
> The next patch introduces a separate JIT-related module with convenient
> utils for JIT engine testing. Considering this change it looks vital to
> make a structured utils distributed module instead of "all in one" Lua
> chunk. As a result the original utils.lua is split into the several

Typo: s/As a result/As a result,/

> modules per subsystem to be tested (e.g. GC, frontend, profilers, etc.).
> 
> Lazy loading of the introduced submodules allows to use this utils in
> all test chunks regardless LuaJIT configuration (e.g. with JIT engine

Typo: s/JIT engine/the JIT engine/

> disabled, without FFI support, etc) and do not spoil utils table with

Typo: s/etc/etc./

> the excess helpers.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Sergey, considering the changes you've made in the second patch, I
> propose to finally split utils.lua into submodules that loads lazily.
> I've pushed my commit on your branch prior to your patchset. If you have
> some notes regarding this, please share them. Otherwise, I'll push this
> into the LuaJIT long-term branches.
> 
> 
> You can also find the trivial changes made within your commits below:
> 
> ================================================================================
> 
> diff --git a/test/tarantool-tests/lj-981-folding-0.test.lua b/test/tarantool-tests/lj-981-folding-0.test.lua
> index 64473ba3..d156f53d 100644
> --- a/test/tarantool-tests/lj-981-folding-0.test.lua
> +++ b/test/tarantool-tests/lj-981-folding-0.test.lua

<snipped>

> diff --git a/test/tarantool-tests/unit-jit-parse.test.lua b/test/tarantool-tests/unit-jit-parse.test.lua
> index e9c0bb80..e4445bf4 100644
> --- a/test/tarantool-tests/unit-jit-parse.test.lua
> +++ b/test/tarantool-tests/unit-jit-parse.test.lua

<snipped>

> ================================================================================
> 
>  test/tarantool-tests/CMakeLists.txt           |   2 +-
>  .../bc-jit-unpatching.test.lua                |   5 +-
>  .../fix-gc-setupvalue.test.lua                |   4 +-
>  .../gh-4427-ffi-sandwich.test.lua             |   2 +-
>  .../gh-5813-resolving-of-c-symbols.test.lua   |   2 +-
>  ...-missed-carg1-in-bctsetr-fallback.test.lua |   2 +-
>  .../lj-351-print-tostring-number.test.lua     |   2 +-
>  .../lj-586-debug-non-string-error.test.lua    |   2 +-
>  .../lj-flush-on-trace.test.lua                |   2 +-
>  .../misclib-getmetrics-lapi.test.lua          |   2 +-
>  .../misclib-memprof-lapi.test.lua             |   2 +-
>  .../misclib-sysprof-lapi.test.lua             |   2 +-
>  test/tarantool-tests/utils.lua                | 125 ------------------
>  test/tarantool-tests/utils/exec.lua           |  52 ++++++++
>  test/tarantool-tests/utils/frontend.lua       |  25 ++++
>  test/tarantool-tests/utils/gc.lua             |  33 +++++
>  test/tarantool-tests/utils/init.lua           |   7 +
>  test/tarantool-tests/utils/jit/const.lua      |   8 ++
>  test/tarantool-tests/utils/jit/init.lua       |   7 +
>  test/tarantool-tests/utils/tools.lua          |  15 +++
>  20 files changed, 162 insertions(+), 139 deletions(-)
>  delete mode 100644 test/tarantool-tests/utils.lua
>  create mode 100644 test/tarantool-tests/utils/exec.lua
>  create mode 100644 test/tarantool-tests/utils/frontend.lua
>  create mode 100644 test/tarantool-tests/utils/gc.lua
>  create mode 100644 test/tarantool-tests/utils/init.lua
>  create mode 100644 test/tarantool-tests/utils/jit/const.lua
>  create mode 100644 test/tarantool-tests/utils/jit/init.lua
>  create mode 100644 test/tarantool-tests/utils/tools.lua
> 
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 527905b6..14a98cf2 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -75,7 +75,7 @@ add_subdirectory(lj-flush-on-trace)
>  # directory), so LUA_PATH need to be updated.
>  make_lua_path(LUA_PATH
>    PATHS
> -    ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
> +    ${CMAKE_CURRENT_SOURCE_DIR}/?/init.lua

Side note: see updates in the second mail.

>      ${PROJECT_SOURCE_DIR}/tools/?.lua
>      ${LUAJIT_SOURCE_DIR}/?.lua
>      ${LUAJIT_BINARY_DIR}/?.lua
> diff --git a/test/tarantool-tests/bc-jit-unpatching.test.lua b/test/tarantool-tests/bc-jit-unpatching.test.lua

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/3] test: split utils.lua into several modules

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review! I fixed your nits and proceed with the changes.

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 0/2] Fix canonicalization of +-0.0 keys for IR_NEWREF.

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 10.05.23, Sergey Kaplun via Tarantool-patches wrote:
> The backported commit needs to be checked precisely -- we don't want to
> just check that the assertion failures are gone -- some commits [1] may
> replace the aforementioned assertions with `return 0` or something else
> [2] due to other issues. Hence, we need to be sure that there is no such
> IR as "NEWREF.*-0" for these traces anymore. So the first commit
> introduces new utility module for tests to parse traces dumped into the
> file. There is nothing too fancy -- for now, the returned traces may
> only say is there some particular IR or no. Any other features may be
> added in the future uses, if we need.
> 
> Q: Can we use `jit.dump()` as is, without temporary file?
> A: Yes, but no:
>    `jit.dump()` may be easily patched to use a custom object as the
>    second argument, not only file name. But this module dumps trace
>    information not by lines but more iterative. So we can use the
>    similar approach as in <luajit-gdb.py> -- concat each line and only
>    then dump it. But I don't ready to discuss this opportunity with Mike
>    now:). So, for now just use a temporary file and remove it after
>    usage.
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-noticket-folding-0
> Taranrool PR: https://github.com/tarantool/tarantool/pull/8634
> Related issues:
> * https://github.com/tarantool/tarantool/issues/8516
> * https://github.com/LuaJIT/LuaJIT/issues/981
> 
> [1]: https://github.com/LuaJIT/LuaJIT/commit/d4c0c6e17ef7edf1f2893bc807746b80612e63e9
> [2]: https://github.com/LuaJIT/LuaJIT/issues/994
> 
> Mike Pall (1):
>   Fix canonicalization of +-0.0 keys for IR_NEWREF.
> 
> Sergey Kaplun (1):
>   test: add utility for parsing `jit.dump`
> 
>  src/lj_record.c                               |   2 +
>  .../tarantool-tests/lj-981-folding-0.test.lua |  57 +++++++
>  test/tarantool-tests/utils/jit_parse.lua      | 156 ++++++++++++++++++
>  3 files changed, 215 insertions(+)
>  create mode 100644 test/tarantool-tests/lj-981-folding-0.test.lua
>  create mode 100644 test/tarantool-tests/utils/jit_parse.lua
> 
> -- 
> 2.34.1
> 

-- 
Best regards,
IM