Hi, Sergey! Thanks for the patch! LGTM, except for a few nits below and the single question. >  >>From: Mike Pall >> >>Reported by Yichun Zhang. >> >>(cherry picked from commit 850f8c59d3d04a9847f21f32a6c36d8269b5b6b1) >> >>The `ASMREF_L` reference is defined as `REF_NIL`, so it isn't considered >>as 64 bit address. On GC64 mode it may lead to the following assembly: >Typo: s/as 64 bit/a 64-bit/ >>| mov eax, edi >>so, high 32 bits of the reference are lost. >Typo: s/high/the high/ >> >>This patch adds `IRT_NIL` to `IRT_IS64` mask, to consider `ASMREF_L` >>64 bit long. Now the resulting assembly is the following: >>| mov rax, rdi >> >>False-positive `if` condition in is OK, since `op12` >>already initialized as 0. >> >>False-positive `if` condition in , , >> is OK, since `REF_NIL` is the last reference before >>`REF_BASE` and this iteration of a cycle is still the last one. >> >>Sergey Kaplun: >>* added the description and the test for the problem >> >>Part of tarantool/tarantool#8516 >>--- >> >>Branch: https://github.com/tarantool/luajit/tree/skaplun/or-144-gc64-asmref-l >>Related issues: >>* https://github.com/openresty/lua-resty-core/issues/144 >>* https://github.com/tarantool/tarantool/issues/8516 >>PR: https://github.com/tarantool/tarantool/pull/8553 >>ML: https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode >> >>Don't restrict test case by GC64 mode, because want to test `IR_LREF` >>for any mode. Keep GC64 in the test name, to be clear where expect the >>SegFault. >> >> src/lj_asm.c | 1 + >> src/lj_ir.h | 4 ++- >> src/lj_opt_sink.c | 1 + >> .../or-144-gc64-asmref-l.test.lua | 28 +++++++++++++++++++ >> 4 files changed, 33 insertions(+), 1 deletion(-) >> create mode 100644 test/tarantool-tests/or-144-gc64-asmref-l.test.lua >> >>diff --git a/src/lj_asm.c b/src/lj_asm.c >>index a154547b..fd31cd04 100644 >>--- a/src/lj_asm.c >>+++ b/src/lj_asm.c >>@@ -2013,6 +2013,7 @@ static void asm_setup_regsp(ASMState *as) >>     ir->prev = REGSP_INIT; >>     if (irt_is64(ir->t) && ir->o != IR_KNULL) { >> #if LJ_GC64 >>+ /* The false-positive of irt_is64() for ASMREF_L (REF_NIL) is OK here. */ >>       ir->i = 0; /* Will become non-zero only for RIP-relative addresses. */ >> #else >>       /* Make life easier for backends by putting address of constant in i. */ >>diff --git a/src/lj_ir.h b/src/lj_ir.h >>index 4bad47ed..e8bca275 100644 >>--- a/src/lj_ir.h >>+++ b/src/lj_ir.h >>@@ -375,10 +375,12 @@ typedef struct IRType1 { uint8_t irt; } IRType1; >> #define irt_isint64(t) (irt_typerange((t), IRT_I64, IRT_U64)) >>  >> #if LJ_GC64 >>+/* Include IRT_NIL, so IR(ASMREF_L) (aka REF_NIL) is considered 64 bit. */ >> #define IRT_IS64 \ >>   ((1u<>    (1u<>- (1u<>+ (1u<>+ (1u<> #elif LJ_64 >> #define IRT_IS64 \ >>   ((1u<>diff --git a/src/lj_opt_sink.c b/src/lj_opt_sink.c >>index 929ccb61..a16d112f 100644 >>--- a/src/lj_opt_sink.c >>+++ b/src/lj_opt_sink.c >>@@ -219,6 +219,7 @@ static void sink_sweep_ins(jit_State *J) >>   for (ir = IR(J->cur.nk); ir < irbase; ir++) { >>     irt_clearmark(ir->t); >>     ir->prev = REGSP_INIT; >>+ /* The false-positive of irt_is64() for ASMREF_L (REF_NIL) is OK here. */ >>     if (irt_is64(ir->t) && ir->o != IR_KNULL) >>       ir++; >>   } >>diff --git a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua >>new file mode 100644 >>index 00000000..0c352c29 >>--- /dev/null >>+++ b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua >>@@ -0,0 +1,28 @@ >>+local tap = require('tap') >>+local test = tap.test('or-144-gc64-asmref-l'):skipcond({ >>+ ['Test requires JIT enabled'] = not jit.status(), >>+}) >>+ >>+test:plan(1) >>+ >>+-- Test file to demonstrate LuaJIT `IR_LREF` assembling incorrect >>+-- behaviour. >>+-- See also: >>+-- * https://github.com/openresty/lua-resty-core/issues/144 . >>+-- * https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode . >>+ >>+jit.opt.start('hotloop=1') >>+ >>+local global_env >>+local _ >>+for i = 1, 4 do >>+ -- Test `IR_LREF` assembling: using `ASMREF_L` (`REF_NIL`). >>+ global_env = getfenv(0) >>+ -- Need to reuse the register, to cause emitting of `mov` >>+ -- instruction (see `ra_left()` in ). >>+ _ = tostring(i) >>+end >>+ >>+test:ok(global_env == getfenv(0), 'IR_LREF assembling correctness') >>+ >>+os.exit(test:check() and 0 or 1) >Neither this test case, nor the original one from OpenResty fail before the patch on OSX/ARM64. >Is it expected behavior or not? >On x86 GC64 it behaves as expected though. >>-- >>2.34.1 >-- >Best regards, >Maxim Kokryashkin >