Skip to content

Commit 52858bc

Browse files
authored
Merge pull request #70 from ivbondarev/fix_rindex_tailcall_recording
Fix rindex tail call recording
2 parents 57c59ee + 2265a15 commit 52858bc

File tree

4 files changed

+124
-9
lines changed

4 files changed

+124
-9
lines changed

src/jit/lj_ffrecord.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,6 +1335,16 @@ static void recff_ujit_table_rindex(jit_State *J, RecordFFData *rd)
13351335
tr = emitir(IRT(IR_TVARGF, IRT_PTR), tr, J->maxslot);
13361336

13371337
J->base[0] = lj_ir_call(J, IRCALL_lj_tab_rawrindex_jit, tr);
1338+
/*
1339+
* By convention, any of returned values set to NULL is a signal
1340+
* for a side exit.
1341+
*/
1342+
emitir(IRTG(IR_NE, IRT_PTR), J->base[0], lj_ir_kkptr(J, NULL));
1343+
/*
1344+
* TVLOAD is emitted with temporary NIL type, will be fixed
1345+
* later - see LJ_POST_FFSPECRET implementation.
1346+
*/
1347+
emitir(IRTG(IR_TVLOAD, IRT_NIL), J->base[0], 0);
13381348
J->postproc = LJ_POST_FFSPECRET;
13391349
} else {
13401350
recff_nyiu(J);

src/jit/lj_record.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,25 +1801,34 @@ void lj_record_ins(jit_State *J)
18011801
{
18021802
/*
18031803
* Fix-up for fast functions that do not have a "fixed" return type.
1804-
* By convention, any of returned values set to NULL is a signal for a
1805-
* side exit. Current implementation handles only cases when exactly one
1806-
* value is returned -- generalize on demand.
1804+
* Current implementation handles only cases when exactly one value
1805+
* is returned -- generalize on demand.
18071806
*/
18081807
BCIns ins = *(J->pc - 1);
18091808
BCReg s;
18101809
const TValue *tv = J->L->base;
1810+
const uint8_t is_tailcall = IR(J->cur.nins - 1)->o == IR_RETF;
18111811

18121812
if (!tvistruecond(&J2G(J)->tmptv2))
18131813
lj_trace_err(J, LJ_TRERR_GFAIL);
18141814

1815-
lua_assert(bc_op(ins) == BC_CALL ||
1816-
bc_op(ins) == BC_CALLT ||
1817-
bc_op(ins) == BC_CALLM ||
1818-
bc_op(ins) == BC_CALLMT);
1815+
if (bc_op(ins) == BC_HOTCNT)
1816+
ins = *(J->pc - 2);
1817+
1818+
/*
1819+
* In case of tail call, ins will point to CALL from caller function,
1820+
* not to the original CALLT/CALLMT made in callee.
1821+
*/
1822+
lua_assert(bc_op(ins) == BC_CALL || bc_op(ins) == BC_CALLM);
18191823

18201824
s = bc_a(ins);
1821-
emitir(IRTG(IR_NE, IRT_PTR), J->base[s], lj_ir_kkptr(J, NULL));
1822-
J->base[s] = emitir(IRTG(IR_TVLOAD, itype2irt(&tv[s])), J->base[s], 0);
1825+
1826+
IRRef ref_tvload = J->cur.nins - is_tailcall - 1;
1827+
IRIns *ins_tvload = IR(ref_tvload);
1828+
lua_assert(ins_tvload->o == IR_TVLOAD);
1829+
ins_tvload->t.irt = IRT_GUARD | itype2irt(&tv[s]);
1830+
1831+
J->base[s] = TREF(ref_tvload, ins_tvload->t.irt);
18231832
}
18241833
/* fallthrough */
18251834
case LJ_POST_FFRETRY: /* Suppress recording of retried fast function. */
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
do --- trace should not return to lower frame before checking rindex return value
2+
local function foo(call_rindex, tab)
3+
if call_rindex == false then return nil end
4+
return ujit.table.rindex(tab, "k1", "k2")
5+
end
6+
7+
local function bar(tab)
8+
local ret = foo(true, tab)
9+
-- compiled cycle just to link trace from foo() here
10+
-- otherwise it will leave bar()
11+
for _ = 1, 5 do end
12+
-- stop further recordings
13+
print("nyi bytecode")
14+
return ret
15+
end
16+
17+
foo(false, {})
18+
foo(false, {})
19+
foo(false, {})
20+
foo(false, {})
21+
foo(false, {})
22+
foo(false, {}) -- record only nil branch
23+
24+
local tab = {k1 = {k2 = "str1"}}
25+
bar(tab)
26+
-- side trace which records rindex and will stop in bar()
27+
bar(tab)
28+
-- trace should exit correctly to start of foo() without unwinding to bar() frame
29+
assert(bar({k1 = {k2 = 42}}) == 42)
30+
end
31+
32+
do --- rindex return value check must not be broken
33+
local function foo(call_rindex, tab)
34+
if call_rindex == false then return nil end
35+
return ujit.table.rindex(tab, "k1", "k2")
36+
end
37+
38+
local function bar(tab)
39+
foo(true, tab)
40+
for _ = 1, 5 do end
41+
print("nyi bytecode")
42+
end
43+
44+
foo(false, {})
45+
foo(false, {})
46+
foo(false, {})
47+
foo(false, {})
48+
foo(false, {})
49+
foo(false, {})
50+
51+
local tab = {k1 = {k2 = {}}}
52+
bar(tab)
53+
bar(tab)
54+
bar(tab) -- should exit normally
55+
end
56+
57+
do --- ensure correct HOTCNT handling (i.e. applying LJ_POST_FFSPECRET while J->pc-1 points to HOTCNT)
58+
local function foo(tab)
59+
return ujit.table.rindex(tab, "k1", "k2")
60+
end
61+
62+
local function bar(tab)
63+
for _ = 1, 5 do
64+
foo(tab)
65+
end
66+
end
67+
68+
bar({k1 = {k2 = false}})
69+
end

tests/impl/uJIT-tests-Lua/suite/table.t

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,4 +285,31 @@ $tester->run('rindex/no-recording.lua', args => '-p-')
285285
->stdout_has('asynchronous abort')
286286
;
287287

288+
$tester->run('rindex/tailcall.lua', args => '-Ohotloop=3 -Ohotexit=2 -p-')
289+
->exit_ok()
290+
->stdout_matches(qr/.+TRACE.3.IR.+\s
291+
0024.rax.+p64.CALLL.+lj_tab_rawrindex_jit.+\(0023\)\s
292+
0025.+>.+p64.NE.+0024.+\[NULL\]\s
293+
0026.+rax.+>.+str.TVLOAD.0024\s
294+
0027.+>.+p32.RETF\s
295+
.+SNAP.+\#1\s
296+
.+TRACE.3.mcode.+/xs)
297+
->stdout_matches(qr/.+TRACE.6.IR.+\s
298+
0024.rax.+p64.CALLL.+lj_tab_rawrindex_jit.+\(0023\)\s
299+
0025.+>.+p64.NE.+0024.+\[NULL\]\s
300+
0026.+>.+tab.TVLOAD.0024\s
301+
0027.+>.+p32.RETF\s
302+
.+SNAP.+\#1\s
303+
.+TRACE.6.mcode.+/xs)
304+
->stdout_matches(qr/.+TRACE.7.IR.+\s
305+
0025.rax.+p64.CALLL.+lj_tab_rawrindex_jit.+\(0024\)\s
306+
0026.+>.+p64.NE.+0025.+\[NULL\]\s
307+
0027.+>.+fal.TVLOAD.0025\s
308+
.+LOOP.+\s
309+
0034.rax.+p64.CALLL.+lj_tab_rawrindex_jit.+\(0033\)\s
310+
0035.+>.+p64.NE.+0034.+\[NULL\]\s
311+
0036.+>.+fal.TVLOAD.0034\s
312+
.+TRACE.7.mcode.+/xs)
313+
;
314+
288315
exit;

0 commit comments

Comments
 (0)