[Tarantool-patches] [PATCH luajit] Fix jit.dump() output for IR_CONV.

[Tarantool-patches] [PATCH luajit] Fix jit.dump() output for IR_CONV.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

(cherry-picked from commit 44bd7437a27e0d19bcf878c20ad27a673f17f40b)

When dumping IRs via `jit.dump()`, the incorrect value of `IR_CONV` mode
shift (`IRCONV_CSH`) is used. This patch fixes the value.

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

Part of tarantool/tarantool#9145
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-ir-conv
Tarantool PR: https://github.com/tarantool/tarantool/pull/9307
Issue: https://github.com/tarantool/tarantool/issues/9145

 src/jit/dump.lua                              |  2 +-
 .../fix-jit-dump-ir-conv.test.lua             | 68 +++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/fix-jit-dump-ir-conv.test.lua

diff --git a/src/jit/dump.lua b/src/jit/dump.lua
index 27b5c2ae..84fe0044 100644
--- a/src/jit/dump.lua
+++ b/src/jit/dump.lua
@@ -277,7 +277,7 @@ local litname = {
     local s = irtype[band(mode, 31)]
     s = irtype[band(shr(mode, 5), 31)].."."..s
     if band(mode, 0x800) ~= 0 then s = s.." sext" end
-    local c = shr(mode, 14)
+    local c = shr(mode, 12)
     if c == 2 then s = s.." index" elseif c == 3 then s = s.." check" end
     t[mode] = s
     return s
diff --git a/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua b/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
new file mode 100644
index 00000000..ed03210a
--- /dev/null
+++ b/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
@@ -0,0 +1,68 @@
+local tap = require('tap')
+local test = tap.test('fix-jit-dump-ir-conv'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
+
+test:plan(2)
+
+-- Test file to demonstrate LuaJIT incorrect `jit.dump()` output
+-- for `IR_CONV`.
+
+local jparse = require('utils').jit.parse
+
+-- XXX: Avoid any traces compilation due to hotcount collisions
+-- for predictable results.
+jit.off()
+jit.flush()
+
+jit.opt.start('hotloop=1')
+
+jit.on()
+jparse.start('i')
+
+-- luacheck: ignore
+local tab = {}
+local idx = 1
+for _ = 1, 4 do
+  -- `int IR_CONV int.num index`.
+  tab[idx] = idx
+end
+
+local traces = jparse.finish()
+
+-- Skip tests for DUALNUM mode since it has no conversions (for
+-- the same cases).
+local IS_DUALNUM = not traces[1]:has_ir('num SLOAD')
+
+test:ok(IS_DUALNUM or traces[1]:has_ir('CONV.*int.num index'),
+        'correct dump for index')
+
+local function trace(step)
+  -- `int IR_CONV int.num check` for `step` conversion.
+  for _ = 1, 4, step do
+  end
+end
+
+-- XXX: Reset hotcounters and traces.
+jit.flush()
+jit.opt.start('hotloop=1')
+
+jparse.start('i')
+-- Compile the inner trace first.
+trace(1)
+-- Compile the big trace with the first trace inlined.
+-- Needs narrowing optimization enabled.
+-- XXX: Reset hotcounters and call the function 3 times to
+-- compile. Needed to avoid hotcount collisions.
+jit.opt.start('hotloop=1')
+trace(1)
+trace(1)
+trace(1)
+
+traces = jparse.finish()
+
+test:ok(IS_DUALNUM or traces[2]:has_ir('CONV.*int.num check'),
+        'correct dump for check')
+
+test:done(true)
-- 
2.42.0

Re: [Tarantool-patches] [PATCH luajit] Fix jit.dump() output for IR_CONV.

[Maxim Kokryashkin via Tarantool-patches]

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


Hi, Sergey!
Thanks for the patch!
LGTM
 
--
Best regards,
Maxim Kokryashkin
 
  
>Среда, 25 октября 2023, 16:18 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
> 
>From: Mike Pall <mike>
>
>(cherry-picked from commit 44bd7437a27e0d19bcf878c20ad27a673f17f40b)
>
>When dumping IRs via `jit.dump()`, the incorrect value of `IR_CONV` mode
>shift (`IRCONV_CSH`) is used. This patch fixes the value.
>
>Sergey Kaplun:
>* added the description and the test for the problem
>
>Part of tarantool/tarantool#9145
>---
>
>Branch:  https://github.com/tarantool/luajit/tree/skaplun/fix-ir-conv
>Tarantool PR:  https://github.com/tarantool/tarantool/pull/9307
>Issue:  https://github.com/tarantool/tarantool/issues/9145
>
> src/jit/dump.lua | 2 +-
> .../fix-jit-dump-ir-conv.test.lua | 68 +++++++++++++++++++
> 2 files changed, 69 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
>
>diff --git a/src/jit/dump.lua b/src/jit/dump.lua
>index 27b5c2ae..84fe0044 100644
>--- a/src/jit/dump.lua
>+++ b/src/jit/dump.lua
>@@ -277,7 +277,7 @@ local litname = {
>     local s = irtype[band(mode, 31)]
>     s = irtype[band(shr(mode, 5), 31)].."."..s
>     if band(mode, 0x800) ~= 0 then s = s.." sext" end
>- local c = shr(mode, 14)
>+ local c = shr(mode, 12)
>     if c == 2 then s = s.." index" elseif c == 3 then s = s.." check" end
>     t[mode] = s
>     return s
>diff --git a/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua b/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
>new file mode 100644
>index 00000000..ed03210a
>--- /dev/null
>+++ b/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
>@@ -0,0 +1,68 @@
>+local tap = require('tap')
>+local test = tap.test('fix-jit-dump-ir-conv'):skipcond({
>+ ['Test requires JIT enabled'] = not jit.status(),
>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>+})
>+
>+test:plan(2)
>+
>+-- Test file to demonstrate LuaJIT incorrect `jit.dump()` output
>+-- for `IR_CONV`.
>+
>+local jparse = require('utils').jit.parse
>+
>+-- XXX: Avoid any traces compilation due to hotcount collisions
>+-- for predictable results.
>+jit.off()
>+jit.flush()
>+
>+jit.opt.start('hotloop=1')
>+
>+jit.on()
>+jparse.start('i')
>+
>+-- luacheck: ignore
>+local tab = {}
>+local idx = 1
>+for _ = 1, 4 do
>+ -- `int IR_CONV int.num index`.
>+ tab[idx] = idx
>+end
>+
>+local traces = jparse.finish()
>+
>+-- Skip tests for DUALNUM mode since it has no conversions (for
>+-- the same cases).
>+local IS_DUALNUM = not traces[1]:has_ir('num SLOAD')
>+
>+test:ok(IS_DUALNUM or traces[1]:has_ir('CONV.*int.num index'),
>+ 'correct dump for index')
>+
>+local function trace(step)
>+ -- `int IR_CONV int.num check` for `step` conversion.
>+ for _ = 1, 4, step do
>+ end
>+end
>+
>+-- XXX: Reset hotcounters and traces.
>+jit.flush()
>+jit.opt.start('hotloop=1')
>+
>+jparse.start('i')
>+-- Compile the inner trace first.
>+trace(1)
>+-- Compile the big trace with the first trace inlined.
>+-- Needs narrowing optimization enabled.
>+-- XXX: Reset hotcounters and call the function 3 times to
>+-- compile. Needed to avoid hotcount collisions.
>+jit.opt.start('hotloop=1')
>+trace(1)
>+trace(1)
>+trace(1)
>+
>+traces = jparse.finish()
>+
>+test:ok(IS_DUALNUM or traces[2]:has_ir('CONV.*int.num check'),
>+ 'correct dump for check')
>+
>+test:done(true)
>--
>2.42.0
 

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

Re: [Tarantool-patches] [PATCH luajit] Fix jit.dump() output for IR_CONV.

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

On 10/25/23 16:14, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> (cherry-picked from commit 44bd7437a27e0d19bcf878c20ad27a673f17f40b)
>
> When dumping IRs via `jit.dump()`, the incorrect value of `IR_CONV` mode
> shift (`IRCONV_CSH`) is used. This patch fixes the value.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9145
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-ir-conv
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9307
> Issue: https://github.com/tarantool/tarantool/issues/9145
>
>   src/jit/dump.lua                              |  2 +-
>   .../fix-jit-dump-ir-conv.test.lua             | 68 +++++++++++++++++++
>   2 files changed, 69 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
>
> diff --git a/src/jit/dump.lua b/src/jit/dump.lua
> index 27b5c2ae..84fe0044 100644
> --- a/src/jit/dump.lua
> +++ b/src/jit/dump.lua
> @@ -277,7 +277,7 @@ local litname = {
>       local s = irtype[band(mode, 31)]
>       s = irtype[band(shr(mode, 5), 31)].."."..s
>       if band(mode, 0x800) ~= 0 then s = s.." sext" end
> -    local c = shr(mode, 14)
> +    local c = shr(mode, 12)
>       if c == 2 then s = s.." index" elseif c == 3 then s = s.." check" end
>       t[mode] = s
>       return s
> diff --git a/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua b/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
> new file mode 100644
> index 00000000..ed03210a
> --- /dev/null
> +++ b/test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
> @@ -0,0 +1,68 @@
> +local tap = require('tap')
> +local test = tap.test('fix-jit-dump-ir-conv'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
> +})
> +
> +test:plan(2)
> +
> +-- Test file to demonstrate LuaJIT incorrect `jit.dump()` output
> +-- for `IR_CONV`.
> +
> +local jparse = require('utils').jit.parse
> +
> +-- XXX: Avoid any traces compilation due to hotcount collisions
> +-- for predictable results.
> +jit.off()
> +jit.flush()
> +
> +jit.opt.start('hotloop=1')
> +
> +jit.on()
> +jparse.start('i')
> +
> +-- luacheck: ignore
> +local tab = {}
> +local idx = 1
> +for _ = 1, 4 do
> +  -- `int IR_CONV int.num index`.
> +  tab[idx] = idx
> +end
> +
> +local traces = jparse.finish()
> +
> +-- Skip tests for DUALNUM mode since it has no conversions (for
> +-- the same cases).
> +local IS_DUALNUM = not traces[1]:has_ir('num SLOAD')
> +
> +test:ok(IS_DUALNUM or traces[1]:has_ir('CONV.*int.num index'),
> +        'correct dump for index')
> +
> +local function trace(step)
> +  -- `int IR_CONV int.num check` for `step` conversion.
> +  for _ = 1, 4, step do
> +  end
> +end
> +
> +-- XXX: Reset hotcounters and traces.
> +jit.flush()
> +jit.opt.start('hotloop=1')
> +
> +jparse.start('i')
> +-- Compile the inner trace first.
> +trace(1)
> +-- Compile the big trace with the first trace inlined.
> +-- Needs narrowing optimization enabled.
> +-- XXX: Reset hotcounters and call the function 3 times to
> +-- compile. Needed to avoid hotcount collisions.
> +jit.opt.start('hotloop=1')
> +trace(1)
> +trace(1)
> +trace(1)
> +
> +traces = jparse.finish()
> +
> +test:ok(IS_DUALNUM or traces[2]:has_ir('CONV.*int.num check'),
> +        'correct dump for check')
> +
> +test:done(true)

Re: [Tarantool-patches] [PATCH luajit] Fix jit.dump() output for IR_CONV.

[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 25.10.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> (cherry-picked from commit 44bd7437a27e0d19bcf878c20ad27a673f17f40b)
> 
> When dumping IRs via `jit.dump()`, the incorrect value of `IR_CONV` mode
> shift (`IRCONV_CSH`) is used. This patch fixes the value.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9145
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-ir-conv
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9307
> Issue: https://github.com/tarantool/tarantool/issues/9145
> 
>  src/jit/dump.lua                              |  2 +-
>  .../fix-jit-dump-ir-conv.test.lua             | 68 +++++++++++++++++++
>  2 files changed, 69 insertions(+), 1 deletion(-)
>  create mode 100644 test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
> 

<snipped>

> -- 
> 2.42.0
> 

-- 
Best regards,
IM