From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>, Sergey Bronnikov <sergeyb@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit] Maintain chain invariant in DCE. Date: Fri, 17 May 2024 16:29:18 +0300 [thread overview] Message-ID: <20240517132918.25926-1-skaplun@tarantool.org> (raw) From: Mike Pall <mike> Thanks to Peter Cawley. (cherry picked from commit f72c19e482b6f918b7cf42b0436e2b117d160a29) Instructions with strong guards that are sometimes emitted with a guard and sometimes emitted without a guard (like HREFK, CONV, or SLOAD) may be eliminated from the IR chain and replaced with the NOP IR. If the next IR of the same kind on the trace is not eliminated, it may reference the IR NOP instead of an instruction of the same type. This may lead to the corresponding assertion failure in the `rec_check_ir()`. This patch unconditionally links the IRs during chain maintenance in DCE. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#9924 --- Be aware that the reproducer from the ticket does not lead to the assertion failure (this is why it is omitted in the test). I suppose it just illustrates the situation when the IR is left off the chain. Although the reproducer is clumsy, I can't simplify it or make it less tricky. Please, ideas are welcome :). Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1094-ir-chain-dce Related Issues: * https://github.com/tarantool/tarantool/issues/9924 * https://github.com/LuaJIT/LuaJIT/issues/1094 src/lj_opt_dce.c | 2 +- .../lj-1094-ir-chain-dce.test.lua | 51 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/lj-1094-ir-chain-dce.test.lua diff --git a/src/lj_opt_dce.c b/src/lj_opt_dce.c index 2417f324..6948179c 100644 --- a/src/lj_opt_dce.c +++ b/src/lj_opt_dce.c @@ -44,7 +44,6 @@ static void dce_propagate(jit_State *J) IRIns *ir = IR(ins); if (irt_ismarked(ir->t)) { irt_clearmark(ir->t); - pchain[ir->o] = &ir->prev; } else if (!ir_sideeff(ir)) { *pchain[ir->o] = ir->prev; /* Reroute original instruction chain. */ ir->t.irt = IRT_NIL; @@ -53,6 +52,7 @@ static void dce_propagate(jit_State *J) ir->prev = 0; continue; } + pchain[ir->o] = &ir->prev; if (ir->op1 >= REF_FIRST) irt_setmark(IR(ir->op1)->t); if (ir->op2 >= REF_FIRST) irt_setmark(IR(ir->op2)->t); } diff --git a/test/tarantool-tests/lj-1094-ir-chain-dce.test.lua b/test/tarantool-tests/lj-1094-ir-chain-dce.test.lua new file mode 100644 index 00000000..3faae7d4 --- /dev/null +++ b/test/tarantool-tests/lj-1094-ir-chain-dce.test.lua @@ -0,0 +1,51 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect maintenance of the +-- IR chain during DCE. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1094. + +local test = tap.test('lj-1094-ir-chain-dce'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +jit.opt.start('hotloop=1') + +-- XXX: The code below is generated by the fuzzer (locally) and +-- simplified as much as possible. + +-- Simple noop function. +local __newindex = function() end +debug.setmetatable(0, { + __newindex = __newindex, +}) + +local counter = 0 +-- luacheck: no unused +local tab = {} +while true do + -- The loop is still not recorded because the guard always + -- fails. + -- So, just try to compile it and check that there is no + -- assertion failure. + if counter > 2 then break end + counter = counter + 1 + -- The pattern `-#{}` allows us to get CONV IRs. The first + -- appearance of this IR (in the `(-#{}).key`) is considered + -- unused by the compiler due to the corresponding "noop" + -- `__newindex` function. + -- The second usage of conversion (`tab[-#{}]`) is guarded (to + -- the int type), so it can't be eliminated. + -- As a result, the 0048 CONV references the 0039 NOP IR after + -- DCE in the IR chain. + -- XXX: TDUP prevents the corresponding second usage from being + -- eliminated since the table insert semantics may change. + -- XXX: Use some numbers to simplify reading the `jit.dump` + -- output. + tab, tab[-#{}], (-#{}).key = {tdup = 'tdup'}, 1, 2 +end + +test:ok(true, 'no assertion failure') + +test:done(true) -- 2.45.0
next reply other threads:[~2024-05-17 13:33 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-05-17 13:29 Sergey Kaplun via Tarantool-patches [this message] 2024-05-26 9:36 ` Maxim Kokryashkin via Tarantool-patches 2024-05-27 7:25 ` Sergey Kaplun via Tarantool-patches 2024-05-27 8:29 ` Maxim Kokryashkin via Tarantool-patches 2024-06-14 13:47 ` Sergey Bronnikov via Tarantool-patches 2024-06-16 19:35 ` Sergey Kaplun via Tarantool-patches 2024-06-17 8:44 ` Sergey Bronnikov via Tarantool-patches 2024-07-09 8:04 ` 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=20240517132918.25926-1-skaplun@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=m.kokryashkin@tarantool.org \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] Maintain chain invariant in DCE.' \ /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