From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 1A2746EC44; Fri, 16 Oct 2026 17:25:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1A2746EC44 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1792160734; bh=b+GsQvcagA1GTyYom1/w2PnjKcy01fE9d7yXyVmUk6A=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=vKwLCd5npVJM/ft65wMrqq3zHyu3WUNEUQTKqMMF0QiXeYdu8MTC97gx+2MHIl6fc 8r4p97stzeI3KODPYMbBKESY1xTf3ao2RaRAWR9eJCZbk7bBIDDyib9WHtCJOOyIYP KikfICHNfRaGx3bRkCUfBmZDxnZ922wUi/HthHcE= Received: from send240.i.mail.ru (send240.i.mail.ru [95.163.59.79]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4957F6EC44 for ; Fri, 16 Oct 2026 17:25:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4957F6EC44 Received: by exim-smtp-84d6fb566c-v8dqw with esmtpa (envelope-from ) id 1xHisF-00000000C4D-1Okt; Fri, 16 Oct 2026 17:25:31 +0300 Message-ID: <02dcf0b6-e145-4bf4-a9c4-37726f80ea0d@tarantool.org> Date: Fri, 16 Oct 2026 17:25:29 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun , Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org References: Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D5ABD805758C3F0BB3F6B4C14D0ED584CFFFF199AB2EF0CF182A05F538085040AEE4EE70D50442633DE06ABAFEAF6705BFF7DE674BE6128D2F3D11C9BF5D973992EB015F9096E3EE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7CC84CC3AD347B910EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375667E1D4B08D42AEC8740CBE59DEFCAC38D5949272EA1387BEEF7252B919079E24E389733CBF5DBD5E9D5E8D9A59859A8B636DA1BED736F9328CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE74F0F518E68DBD4F843847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5213A7DDBACDC27685002B1117B3ED696ACED3ACB40BE862D19AC5B239BAD4335823CB91A9FED034534781492E4B8EEAD85CCBA673D36D1A4BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE1918E10F71CB4DF9F96AB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D659AC2495DD1A0EFC78E677233A9D40A98D2313EAA619E9611D3C93AC63D67E47208D1C5D8E29AC017DB8341EE9D5BE9A0A2ADD19ED1A832010B1A594029EFBFC26A0C0D56F77F259136536EB022892E5344C41F94D744909CE2512F26BEC029E55448553D2254B8D95CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXOcIETfglQORZ0zpDET4Zrk3igikrdHlWExFoADKRrzZe+eya20PhEY= X-Mailru-Sender: 689FA8AB762F73937C9FA53A4753B3136585059B539380A24A65C3470F15259DC8BF98C9C92888F4EF86D5F70DA33880E41E8EF7A07863ECB274557F927329BE2DDF8182D28ACDB545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Optionally return PC position in jit.util.tracesnap(). X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey, thanks for review! See my comments below. Sergey On 10/13/26 16:41, Sergey Kaplun via Tarantool-patches wrote: > Hi, Sergey! > Thanks for the patch! > LGTM, after fixing my comments below. > > On 08.10.26, Sergey Bronnikov wrote: >> From: Mike Pall >> >> Suggested by Sergey Bronnikov. >> >> (cherry picked from commit 8e6520a7aecd0517e792b359afbbfd7274791f5f) >> >> The optional `getpos` argument is needed for ljopt [1], a tool that >> verifies the correctness of LuaJIT optimizations by translating the >> recorded IR into SMT-LIB and checking the formulas with an SMT solver. >> Traces often lack sufficient information for accurate modeling, so >> ljopt extracts the address of the bytecode pointed to by the snapshot >> directly from LuaJIT during execution. >> >> The LuaJIT internal API includes the function `jit.util.tracesnap(tr, >> sn)`. In principle, this function could be extended to return the >> program counter (pc). This patch does exactly that: an optional third >> argument is added, and when it is truthy the second value returned by >> `jit.util.tracesnap()` is the PC position of the snapshot in the >> bytecode of the function where the trace starts. >> >> Also, the patch adds a test that records a root loop trace and >> checks that the returned position is the `FORL` jump target (the >> loop entry), that no position is returned without the optional >> argument or with a false value, and that every snapshot of the >> trace reports a valid bytecode offset. > I don't understand this paragraph. Usually we don't describe added tests > for the features (especially in the way "what was done" instead of "why > it was done"). removed third paragraph and rephrased the second:     The LuaJIT internal API includes the function     `jit.util.tracesnap(tr, sn)`. The patch adds an optional third     argument, and when it is truthy the second value returned by     `jit.util.tracesnap()` is the PC position of the snapshot in the     bytecode of the function where the trace starts. >> 1. https://github.com/ligurio/ljopt >> > Please, mention the backporting issues as well. Added > >> Sergey Bronnikov: >> * added the description and the test for the feature >> --- >> >> Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-tracesnap-getpos >> >> src/lib_jit.c | 9 +- >> .../lj-1472-tracesnap-getpos.test.lua | 89 +++++++++++++++++++ >> 2 files changed, 97 insertions(+), 1 deletion(-) >> create mode 100644 test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua >> >> diff --git a/src/lib_jit.c b/src/lib_jit.c >> index 9f870f68f..5a0d38611 100644 >> --- a/src/lib_jit.c >> +++ b/src/lib_jit.c > > >> diff --git a/test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua b/test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua >> new file mode 100644 >> index 000000000..a56a7a679 >> --- /dev/null >> +++ b/test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua >> @@ -0,0 +1,89 @@ >> +local tap = require('tap') >> +local bit = require('bit') >> +local jutil = require('jit.util') >> +local vmdef = require('jit.vmdef') >> + >> +-- The test checks the optional `getpos` argument of >> +-- `jit.util.tracesnap()` that makes the function return the PC >> +-- position of the snapshot in the function bytecode. >> +-- See also https://github.com/LuaJIT/LuaJIT/issues/1472. >> + >> +local test = tap.test('lj-1472-tracesnap-getpos'):skipcond({ >> + ['Test requires JIT enabled'] = not jit.status(), >> +}) >> + >> +test:plan(8) >> + >> +-- Entries in `vmdef.bcnames` are 6 characters long. >> +local BC_NAME_LENGTH = 6 >> + >> +-- Return the name of the bytecode operation for the given >> +-- instruction, that is encoded in the low byte. >> +local function opname(ins) >> + local oidx = BC_NAME_LENGTH * bit.band(ins, 0xff) >> + local name = vmdef.bcnames:sub(oidx + 1, oidx + BC_NAME_LENGTH) >> + return (name:gsub('%s+$', '')) > Minor: I suggest the more readable: > | return name:match('%a+') > Feel free to ignore. this will break testcase: tarantool> a = "FORL  " --- ... tarantool> a:gsub('%a+', '') --- - '  ' - 1 ... tarantool> a:gsub('%s+', '') --- - FORL - 1 ... > >> +end >> + >> +local function payload() >> + local sum = 0 >> + for i = 1, 100 do > Minor: Typo: s/100/4/ Fixed:  end  local function payload()    local sum = 0 -  for i = 1, 100 do +  for i = 1, 4 do      sum = sum + i    end    return sum > >> + sum = sum + i >> + end >> + return sum >> +end >> + >> +-- Find the loop back-edge (FORL) in the function bytecode and >> +-- compute its jump target, that is the first instruction of the > Typo: s/that/which/ Fixed:  -- Find the loop back-edge (FORL) in the function bytecode and --- compute its jump target, that is the first instruction of the +-- compute its jump target, which is the first instruction of the  -- loop body. `pc == 0` is the JCproto header, so real  -- instructions start from `pc == 1`. JIT compilation is disabled  -- while scanning, so this loop is not recorded as a trace. >> +-- loop body. `pc == 0` is the JCproto header, so real >> +-- instructions start from `pc == 1`. JIT compilation is disabled >> +-- while scanning, so this loop is not recorded as a trace. >> +jit.off() >> +local nbc = jutil.funcinfo(payload).bytecodes >> +local loop_entry >> +for pc = 1, nbc - 1 do >> + local ins = jutil.funcbc(payload, pc) >> + if opname(ins) == 'FORL' then >> + -- The jump offset is relative to the next instruction and is >> + -- biased by `BCBIAS_J`. >> + local jmp = bit.rshift(ins, 16) - 0x8000 > Let's name the magic number: > | local BCBIAS_J = 0x8000 Fixed:  jit.off()  local nbc = jutil.funcinfo(payload).bytecodes  local loop_entry +local BCBIAS_J = 0x8000  for pc = 1, nbc - 1 do    local ins = jutil.funcbc(payload, pc)    if opname(ins) == 'FORL' then      -- The jump offset is relative to the next instruction and is      -- biased by `BCBIAS_J`. -    local jmp = bit.rshift(ins, 16) - 0x8000 +    local jmp = bit.rshift(ins, 16) - BCBIAS_J      loop_entry = pc + 1 + jmp      break    end > >> + loop_entry = pc + 1 + jmp >> + break >> + end >> +end >> +test:isnt(loop_entry, nil, 'FORL instruction is found') > It should be an assert instead, since this is the test precondition. > >> + >> +-- Record a root loop trace in `payload`. Snapshot #0 points to >> +-- the loop entry (the FORL jump target). >> +jit.on() >> +jit.flush() >> +jit.opt.start('hotloop=1') >> +payload() >> + >> +local info = jutil.traceinfo(1) >> +test:ok(info, 'the root loop trace is recorded') > It should be an assert instead, since this is the test precondition. Updated:      loop_entry = pc + 1 + jmp      break    end  end -test:isnt(loop_entry, nil, 'FORL instruction is found') +assert(loop_entry, 'FORL instruction is found')  -- Record a root loop trace in `payload`. Snapshot #0 points to  -- the loop entry (the FORL jump target). > >> +local snap, pos = jutil.tracesnap(1, 0, true) >> +test:istable(snap, 'the snapshot table is returned') >> +test:is(pos, loop_entry, 'the PC position points to the loop entry') >> + >> +local _, pos_without = jutil.tracesnap(1, 0) >> +test:is(pos_without, nil, 'no PC position without getpos') >> + >> +local _, pos_false = jutil.tracesnap(1, 0, false) >> +test:is(pos_false, nil, 'no PC position with getpos=false') >> + >> +local all_valid = true >> +for sn = 0, info.nexit - 1 do >> + local _, p = jutil.tracesnap(1, sn, true) >> + if type(p) ~= 'number' or p < 0 or p >= nbc or >> + jutil.funcbc(payload, p) == nil then >> + all_valid = false >> + end >> +end >> +test:ok(all_valid, 'positions of all snapshots are valid offsets') >> + >> +test:is(select('#', jutil.tracesnap(1, info.nexit, true)), 0, >> + 'out-of-range snapshot returns nothing') >> + > It would be nice to add any correctness test. > The most trivial in mind is the following: > > | src/luajit -Ohotloop=1 -e ' > | local jutil = require"jit.util" > | local function trivial() > | local a = 0 > | return a > | end > | trivial() > | trivial() > | > | local _, b = jutil.tracesnap(1, 0, true) > | print(b) > | local _, b = jutil.tracesnap(1, 1, true) > | print(b) > | ' > | 1 > | 2 > > The snapshot is emitted at the function entry and before returning the > result which makes it trivial to detect: > > | ---- TRACE 1 start (command line):3 > | 0001 KSHORT 0 0 > | 0002 RET1 0 2 > | ---- TRACE 1 IR > | .... SNAP #0 [ ---- ] > | .... SNAP #1 [ ---- +0 ] > > Don't forget to reset the hotcounters during the recording of this > example. ok, thanks, added:  }) -test:plan(8) +test:plan(10)  -- Entries in `vmdef.bcnames` are 6 characters long.  local BC_NAME_LENGTH = 6 @@ -87,4 +87,21 @@ test:ok(all_valid, 'positions of all snapshots are valid offsets')  test:is(select('#', jutil.tracesnap(1, info.nexit, true)), 0,          'out-of-range snapshot returns nothing') +-- The same correctness check for a trace without a loop: the +-- first and the last snapshots of the trivial function point to +-- its first and second bytecode instructions, respectively. +jit.flush() +local function trivial() +  local a = 0 +  return a +end +trivial() +trivial() + +local _, pos_first = jutil.tracesnap(1, 0, true) +test:is(pos_first, 1, 'the first snapshot points to the first instruction') + +local _, pos_last = jutil.tracesnap(1, 1, true) +test:is(pos_last, 2, 'the second snapshot points to the second instruction') +  test:done(true) > >> +test:done(true) >> -- >> 2.51.0 >>