Hi, Sergey! Thanks for the patch! LGTM, except for a few nits below.   >  >>From: Mike Pall >> >>Contributed by Peter Cawley. >> >>(cherry picked from commit b4ed3219a1a98dd9fe7d1e3eeea3b82f5a780948) >> >>When emitting `IR_HREF` for constant value to lookup the `ir_khash()` >Typo: s/for constant/for a constant >>function is used to calculate hash for the corresponding object. >Typo: s/hash/the hash >>This calculation must be the same as in the corresponding `hashkey()` >>function from . >> >>Hash calculating via passing two arguments `lo`, and `hi` to `hashrot()` >Typo: s/calculating via/is calculated by >>routine. For non-string GC objects the first `lo` argument is the same >>for GC64 and not GC64 mode -- lower 32 bits of the object address. For >>GC64 mode `hi` argument is upper 32 bits of the object address, >>including specific type NaN-tag. This `hi` argument in `ir_khash()` >>function is miscalculated in GC64 using non-GC64 value (`lo` + >>`HASH_BIAS`). As a result, the hash for the GC object is miscalculated >>on trace and we exit from trace due to assertion guard on the type or >>value check. >> >>This patch fixes calculation of hash value on trace for GC64 mode by >>making it consistent with `hashkey()`. >Typo: s/of hash/of the hash >> >>Sergey Kaplun: >>* added the description and the test for the problem >> >>Part of tarantool/tarantool#7230 >>--- >> >>Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-356-ir-khash-non-string-obj-full-ci >>Issue/PR: >>* https://github.com/tarantool/tarantool/issues/7230 >>* https://github.com/LuaJIT/LuaJIT/pull/356 >>Tarantool PR: https://github.com/tarantool/tarantool/pull/8020 >> >>Side note: Problems with red fuzzer jobs look irrelevant to the patch. >> >> src/lj_asm.c | 4 + >> .../lj-356-ir-khash-non-string-obj.test.lua | 90 +++++++++++++++++++ >> 2 files changed, 94 insertions(+) >> create mode 100644 test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua >> >>diff --git a/src/lj_asm.c b/src/lj_asm.c >>index 1a7fb0c8..a154547b 100644 >>--- a/src/lj_asm.c >>+++ b/src/lj_asm.c >>@@ -1016,7 +1016,11 @@ static uint32_t ir_khash(IRIns *ir) >>   } else { >>     lua_assert(irt_isgcv(ir->t)); >>     lo = u32ptr(ir_kgc(ir)); >>+#if LJ_GC64 >>+ hi = (uint32_t)(u64ptr(ir_kgc(ir)) >> 32) | (irt_toitype(ir->t) << 15); >>+#else >>     hi = lo + HASH_BIAS; >>+#endif >>   } >>   return hashrot(lo, hi); >> } >>diff --git a/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua b/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua >>new file mode 100644 >>index 00000000..fff0b1a5 >>--- /dev/null >>+++ b/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua >>@@ -0,0 +1,90 @@ >>+local tap = require('tap') >>+local traceinfo = require('jit.util').traceinfo >>+local table_new = require('table.new') >>+ >>+-- Test file to demonstrate the incorrect GC64 JIT behaviour >>+-- for `IR_HREF` for on-trace-constant key lookup. >>+-- See also https://github.com/LuaJIT/LuaJIT/pull/356 . >>+local test = tap.test('lj-356-ir-khash-non-string-obj') >>+local N_ITERATIONS = 4 >>+ >>+-- Amount of iteration for trace compilation and execution and >>+-- additional check, that there is no new trace compiled. >>+test:plan(N_ITERATIONS + 1) >>+ >>+-- To reproduce the issue we need to compile a trace with >>+-- `IR_HREF`, with a lookup of constant hash key GC value. To >>+-- prevent `IR_HREFK` to be emitted instead, we need a table with >>+-- a huge hash part. Delta of address between the start of the >>+-- hash part of the table and the current node to lookup must be >>+-- more than `(1024 * 64 - 1) * sizeof(Node)`. >>+-- See , for details. >>+-- XXX: This constant is well suited to prevent test to be flaky, >>+-- because the aforementioned delta is always large enough. >>+local N_HASH_FIELDS = 1024 * 1024 * 8 >>+local MAGIC = 42 >>+ >>+local filled_tab = table_new(0, N_HASH_FIELDS + 1) >>+ >>+-- The function returns constant cdata pinned to `GCproto` to be >>+-- used as a key for table lookup. >>+local function get_const_cdata() >>+ return 0LL >>+end >>+ >>+-- XXX: don't set `hotexit` to prevent compilation of trace after >>+-- exiting the main test cycle. >>+jit.opt.start('hotloop=1') >>+ >>+-- Prevent `get_const_cdata()` become hot and be compiled before >>+-- the main test cycle. >Typo: s/become hot and be/from becoming hot and being >>+jit.off() >>+ >>+filled_tab[get_const_cdata()] = MAGIC >>+ >>+-- Speed up table filling-up. >>+jit.on() >>+ >>+-- Filling-up the table with GC values to minimize the amount of >>+-- hash collisions and increases delta between the start of the >Typo: s/increases/increase >>+-- hash part of the table and currently stored node. >>+for i = 1, N_HASH_FI >>%MCEPASTEBIN% >>ELDS do >>+ filled_tab[1LL] = i >>+end >>+ >>+-- Prevent JIT misbehaviour before the main test chunk. >>+jit.off() >>+ >>+-- Allocate a table with exact array part to be sure that there >>+-- is no side exit from the trace, due to table reallocation. >>+local result_tab = table_new(N_ITERATIONS, 0) >>+ >>+jit.flush() >>+ >>+assert(not traceinfo(1), 'no traces compiled after flush') >>+ >>+jit.on() >>+ >>+for _ = 1, N_ITERATIONS do >>+ -- If the hash for table lookup is miscalculated, then we get >>+ -- `nil` (most possibly) value from the table and the side exit >>+ -- will be taken and we continue execution from the call to >>+ -- `get_const_cdata()`, this function is already hot after the >>+ -- first cycle iteration, and the new trace is recorded. >>+ table.insert(result_tab, filled_tab[get_const_cdata()]) >>+end >>+ >>+jit.off() >>+ >>+test:ok(not traceinfo(2), 'the second trace should not be compiled') >>+ >>+-- No more need to prevent trace compilation. >>+jit.on() >>+ >>+for i = 1, N_ITERATIONS do >>+ -- Check that that all lookups are correct and there is no >>+ -- value from other cdata stored in the table. >>+ test:ok(result_tab[i] == MAGIC, 'correct hash lookup from the table') >>+end >>+ >>+os.exit(test:check() and 0 or 1) >>-- >>2.34.1 >-- >Best regards, >Maxim Kokryashkin >