[Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.

[Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Suggested by Sergey Kaplun.

(cherry picked from commit d2fe2a6d465a3e4c74c9876db94ae606f9c6983b)

This patch replaces the number of NYI bytecodes that can't be compiled
with their names in the `jit.dump()` and -jv outputs. Since the
functionality is the same, only `jit.dump()` is tested as most popular.

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

Part of tarantool/tarantool#9924
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-567-1176-print-nyi-names
Related Issues:
* https://github.com/tarantool/tarantool/issues/9924
* https://github.com/LuaJIT/LuaJIT/pull/567
* https://github.com/LuaJIT/LuaJIT/issues/1176

 src/jit/dump.lua                              |  7 +-
 src/jit/v.lua                                 |  9 +-
 src/lj_traceerr.h                             |  2 +-
 .../lj-567-1176-print-nyi-names.test.lua      | 82 +++++++++++++++++++
 4 files changed, 96 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua

diff --git a/src/jit/dump.lua b/src/jit/dump.lua
index 84fe0044..e7f05253 100644
--- a/src/jit/dump.lua
+++ b/src/jit/dump.lua
@@ -543,7 +543,12 @@ local recdepth = 0
 local function fmterr(err, info)
   if type(err) == "number" then
     if type(info) == "function" then info = fmtfunc(info) end
-    err = format(vmdef.traceerr[err], info)
+    local fmt = vmdef.traceerr[err]
+    if fmt == "NYI: bytecode %s" then
+      local oidx = 6 * info
+      info = sub(vmdef.bcnames, oidx+1, oidx+6)
+    end
+    err = format(fmt, info)
   end
   return err
 end
diff --git a/src/jit/v.lua b/src/jit/v.lua
index 934de985..ff4e9c7c 100644
--- a/src/jit/v.lua
+++ b/src/jit/v.lua
@@ -63,7 +63,7 @@ assert(jit.version_num == 20100, "LuaJIT core/library version mismatch")
 local jutil = require("jit.util")
 local vmdef = require("jit.vmdef")
 local funcinfo, traceinfo = jutil.funcinfo, jutil.traceinfo
-local type, format = type, string.format
+local type, sub, format = type, string.sub, string.format
 local stdout, stderr = io.stdout, io.stderr
 
 -- Active flag and output file handle.
@@ -90,7 +90,12 @@ end
 local function fmterr(err, info)
   if type(err) == "number" then
     if type(info) == "function" then info = fmtfunc(info) end
-    err = format(vmdef.traceerr[err], info)
+    local fmt = vmdef.traceerr[err]
+    if fmt == "NYI: bytecode %s" then
+      local oidx = 6 * info
+      info = sub(vmdef.bcnames, oidx+1, oidx+6)
+    end
+    err = format(fmt, info)
   end
   return err
 end
diff --git a/src/lj_traceerr.h b/src/lj_traceerr.h
index 1363c4f3..beecbd8c 100644
--- a/src/lj_traceerr.h
+++ b/src/lj_traceerr.h
@@ -13,7 +13,7 @@ TREDEF(STACKOV,	"trace too deep")
 TREDEF(SNAPOV,	"too many snapshots")
 TREDEF(BLACKL,	"blacklisted")
 TREDEF(RETRY,	"retry recording")
-TREDEF(NYIBC,	"NYI: bytecode %d")
+TREDEF(NYIBC,	"NYI: bytecode %s")
 
 /* Recording loop ops. */
 TREDEF(LLEAVE,	"leaving loop in root trace")
diff --git a/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
new file mode 100644
index 00000000..38829d2c
--- /dev/null
+++ b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
@@ -0,0 +1,82 @@
+local tap = require('tap')
+
+-- Test dumping of names of NYI bytecodes to be compiled instead
+-- of their numbers. See also:
+-- * https://github.com/LuaJIT/LuaJIT/pull/567,
+-- * https://github.com/LuaJIT/LuaJIT/issues/1176.
+
+local test = tap.test('lj-567-1176-print-nyi-names'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
+
+local jparse = require('utils').jit.parse
+
+test:plan(3)
+
+local function reset_jit()
+  -- Remove all previous traces.
+  jit.off()
+  jit.flush()
+  jit.on()
+  jit.opt.start('hotloop=1')
+end
+
+local function nop() end
+local function test_varg(...)
+  for _ = 1, 4 do nop(...) end
+end
+
+local function test_uv()
+  do
+    -- luacheck: ignore
+    local uclo
+    goto close_uv
+    local function _()
+      return uclo
+    end
+  end
+  ::close_uv::
+end
+
+-- We only need the abort reason in the test.
+jparse.start('t')
+reset_jit()
+
+test_varg()
+
+local _, aborted_traces = jparse.finish()
+assert(aborted_traces and aborted_traces[1],
+       'aborted trace with VARG is persisted')
+
+-- We tried to compile only one trace.
+local reason = aborted_traces[1][1].abort_reason
+test:like(reason, 'NYI: bytecode VARG', 'bytecode VARG name')
+
+jparse.start('t')
+reset_jit()
+
+for _ = 1, 4 do
+   local _ = function() end
+end
+_, aborted_traces = jparse.finish()
+assert(aborted_traces and aborted_traces[1],
+       'aborted trace with FNEW is persisted')
+
+reason = aborted_traces[1][1].abort_reason
+test:like(reason, 'NYI: bytecode FNEW', 'bytecode FNEW name')
+
+jparse.start('t')
+reset_jit()
+
+test_uv()
+test_uv()
+
+_, aborted_traces = jparse.finish()
+assert(aborted_traces and aborted_traces[1],
+       'aborted trace with UCLO is persisted')
+
+reason = aborted_traces[1][1].abort_reason
+test:like(reason, 'NYI: bytecode UCLO', 'bytecode UCLO name')
+
+test:done(true)
-- 
2.45.1

Re: [Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM, except for two nits below.

On Wed, Jun 26, 2024 at 11:37:21AM GMT, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Suggested by Sergey Kaplun.
>
> (cherry picked from commit d2fe2a6d465a3e4c74c9876db94ae606f9c6983b)
>
> This patch replaces the number of NYI bytecodes that can't be compiled
I guess `numeric value` is a bit better than `number`, but feel free to
ignore.

> with their names in the `jit.dump()` and -jv outputs. Since the
> functionality is the same, only `jit.dump()` is tested as most popular.
>
> Sergey Kaplun:
> * added the description and the test for the feature
>
> Part of tarantool/tarantool#9924
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-567-1176-print-nyi-names
> Related Issues:
> * https://github.com/tarantool/tarantool/issues/9924
> * https://github.com/LuaJIT/LuaJIT/pull/567
> * https://github.com/LuaJIT/LuaJIT/issues/1176
>
>  src/jit/dump.lua                              |  7 +-
>  src/jit/v.lua                                 |  9 +-
>  src/lj_traceerr.h                             |  2 +-
>  .../lj-567-1176-print-nyi-names.test.lua      | 82 +++++++++++++++++++
>  4 files changed, 96 insertions(+), 4 deletions(-)
>  create mode 100644 test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
>
> diff --git a/src/jit/dump.lua b/src/jit/dump.lua
> index 84fe0044..e7f05253 100644
> --- a/src/jit/dump.lua
> +++ b/src/jit/dump.lua
<snipped>

> diff --git a/src/jit/v.lua b/src/jit/v.lua
> index 934de985..ff4e9c7c 100644
> --- a/src/jit/v.lua
> +++ b/src/jit/v.lua
<snipped>

> diff --git a/src/lj_traceerr.h b/src/lj_traceerr.h
> index 1363c4f3..beecbd8c 100644
> --- a/src/lj_traceerr.h
> +++ b/src/lj_traceerr.h
<snipped>

> diff --git a/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
> new file mode 100644
> index 00000000..38829d2c
> --- /dev/null
> +++ b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
> @@ -0,0 +1,82 @@
> +local tap = require('tap')
> +
> +-- Test dumping of names of NYI bytecodes to be compiled instead
> +-- of their numbers. See also:
> +-- * https://github.com/LuaJIT/LuaJIT/pull/567,
> +-- * https://github.com/LuaJIT/LuaJIT/issues/1176.
> +
> +local test = tap.test('lj-567-1176-print-nyi-names'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
> +})
> +
> +local jparse = require('utils').jit.parse
> +
> +test:plan(3)
> +
> +local function reset_jit()
> +  -- Remove all previous traces.
> +  jit.off()
> +  jit.flush()
> +  jit.on()
> +  jit.opt.start('hotloop=1')
> +end
> +
> +local function nop() end
> +local function test_varg(...)
> +  for _ = 1, 4 do nop(...) end
> +end
> +
> +local function test_uv()
> +  do
> +    -- luacheck: ignore
> +    local uclo
> +    goto close_uv
> +    local function _()
> +      return uclo
> +    end
> +  end
> +  ::close_uv::
> +end
> +
> +-- We only need the abort reason in the test.
> +jparse.start('t')
> +reset_jit()
> +
> +test_varg()
> +
> +local _, aborted_traces = jparse.finish()
> +assert(aborted_traces and aborted_traces[1],
> +       'aborted trace with VARG is persisted')
> +
> +-- We tried to compile only one trace.
> +local reason = aborted_traces[1][1].abort_reason
> +test:like(reason, 'NYI: bytecode VARG', 'bytecode VARG name')
> +
> +jparse.start('t')
> +reset_jit()
> +
> +for _ = 1, 4 do
> +   local _ = function() end
> +end
Maybe it would be better to create a separate function for FNEW too,
just for the uniformity, but that's not a big deal. Feel free to ignore.

> +_, aborted_traces = jparse.finish()
> +assert(aborted_traces and aborted_traces[1],
> +       'aborted trace with FNEW is persisted')
> +
> +reason = aborted_traces[1][1].abort_reason
> +test:like(reason, 'NYI: bytecode FNEW', 'bytecode FNEW name')
> +
> +jparse.start('t')
> +reset_jit()
> +
> +test_uv()
> +test_uv()
> +
> +_, aborted_traces = jparse.finish()
> +assert(aborted_traces and aborted_traces[1],
> +       'aborted trace with UCLO is persisted')
> +
> +reason = aborted_traces[1][1].abort_reason
> +test:like(reason, 'NYI: bytecode UCLO', 'bytecode UCLO name')
> +
> +test:done(true)
> --
> 2.45.1
>

Re: [Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.

[Sergey Kaplun via Tarantool-patches]

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

On 01.07.24, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for two nits below.
> 
> On Wed, Jun 26, 2024 at 11:37:21AM GMT, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Suggested by Sergey Kaplun.
> >
> > (cherry picked from commit d2fe2a6d465a3e4c74c9876db94ae606f9c6983b)
> >
> > This patch replaces the number of NYI bytecodes that can't be compiled
> I guess `numeric value` is a bit better than `number`, but feel free to
> ignore.

Fixed, thanks!

> 
> > with their names in the `jit.dump()` and -jv outputs. Since the
> > functionality is the same, only `jit.dump()` is tested as most popular.
> >
> > Sergey Kaplun:
> > * added the description and the test for the feature
> >
> > Part of tarantool/tarantool#9924
> > ---
> >
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-567-1176-print-nyi-names
> > Related Issues:
> > * https://github.com/tarantool/tarantool/issues/9924
> > * https://github.com/LuaJIT/LuaJIT/pull/567
> > * https://github.com/LuaJIT/LuaJIT/issues/1176
> >
> >  src/jit/dump.lua                              |  7 +-
> >  src/jit/v.lua                                 |  9 +-
> >  src/lj_traceerr.h                             |  2 +-
> >  .../lj-567-1176-print-nyi-names.test.lua      | 82 +++++++++++++++++++
> >  4 files changed, 96 insertions(+), 4 deletions(-)
> >  create mode 100644 test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua

<snipped>

> > diff --git a/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
> > new file mode 100644
> > index 00000000..38829d2c
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua

<snipped>

> > +
> > +jparse.start('t')
> > +reset_jit()
> > +
> > +for _ = 1, 4 do
> > +   local _ = function() end
> > +end
> Maybe it would be better to create a separate function for FNEW too,
> just for the uniformity, but that's not a big deal. Feel free to ignore.

Fixed, see the iterative patch below:

===================================================================
diff --git a/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
index 38829d2c..3d29c6bf 100644
--- a/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
+++ b/test/tarantool-tests/lj-567-1176-print-nyi-names.test.lua
@@ -27,6 +27,12 @@ local function test_varg(...)
   for _ = 1, 4 do nop(...) end
 end
 
+local function test_fnew()
+  for _ = 1, 4 do
+    local _ = function() end
+  end
+end
+
 local function test_uv()
   do
     -- luacheck: ignore
@@ -56,9 +62,8 @@ test:like(reason, 'NYI: bytecode VARG', 'bytecode VARG name')
 jparse.start('t')
 reset_jit()
 
-for _ = 1, 4 do
-   local _ = function() end
-end
+test_fnew()
+
 _, aborted_traces = jparse.finish()
 assert(aborted_traces and aborted_traces[1],
        'aborted trace with FNEW is persisted')
===================================================================

> 

<snipped>

> > --
> > 2.45.1
> >

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey,


On 26.06.2024 11:37, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Suggested by Sergey Kaplun.
>
> (cherry picked from commit d2fe2a6d465a3e4c74c9876db94ae606f9c6983b)
>
> This patch replaces the number of NYI bytecodes that can't be compiled
> with their names in the `jit.dump()` and -jv outputs. Since the
> functionality is the same, only `jit.dump()` is tested as most popular.
>
> Sergey Kaplun:
> * added the description and the test for the feature
>
> Part of tarantool/tarantool#9924
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-567-1176-print-nyi-names
> Related Issues:
> *https://github.com/tarantool/tarantool/issues/9924
> *https://github.com/LuaJIT/LuaJIT/pull/567
> *https://github.com/LuaJIT/LuaJIT/issues/1176

Thanks for the patch! LGTM


<snipped>


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

Re: [Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.

[Sergey Kaplun via Tarantool-patches]

I've checked the patch into all long-term branches in
tarantool/luajit and bumped a new version in master [1], release/3.1 [2]
and release/2.11 [3].

[1]: https://github.com/tarantool/tarantool/pull/10200
[2]: https://github.com/tarantool/tarantool/pull/10201
[3]: https://github.com/tarantool/tarantool/pull/10202

-- 
Best regards,
Sergey Kaplun