Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.
Date: Mon, 1 Jul 2024 12:33:38 +0300	[thread overview]
Message-ID: <ZoJ38t4W-QHEbU-w@root> (raw)
In-Reply-To: <a3ez5msyes5xr3ecekmza7fnr4og7skzos4ohzucqahex5osaa@ndzhqj3zyt65>

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

  reply	other threads:[~2024-07-01  9:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-26  8:37 Sergey Kaplun via Tarantool-patches
2024-07-01  8:36 ` Maxim Kokryashkin via Tarantool-patches
2024-07-01  9:33   ` Sergey Kaplun via Tarantool-patches [this message]
2024-07-04  6:38 ` Sergey Bronnikov via Tarantool-patches
2024-07-09  8:07 ` Sergey Kaplun via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZoJ38t4W-QHEbU-w@root \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] Show name of NYI bytecode in -jv and -jdump.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox