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

[Sergey Bronnikov]

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