Hi, Sergey!
thanks for the fix! Bug is reproduced without fix (I've used incorrect C flag).
LGTM
Sergey
Hi, Sergey! Thanks for the review! See my answers below. On 04.03.26, Sergey Bronnikov wrote:Hi, Sergey, thanks for the patch! See my comments. Sergey On 3/2/26 10:52, Sergey Kaplun wrote:From: Mike Pall <mike> Reported by Sergey Kaplun. (cherry picked from commit b1cd2f83b5d085bb71368b87c91a461be77d4364) `lj_opt_narrow_unm()` in the DUALNUM mode narrows doubles too optimistic, missing 0 check. In that case, the narrowing of 0 is incorrect. This leads to the assertion failure in `rec_check_slots()` for the string obtained from the corresponding number. This patch fixes it by restricting the check of the given TValue. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#12134 --- src/lj_opt_narrow.c | 4 +- ...lj-1418-dualnum-narrowing-minus-0.test.lua | 49 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua diff --git a/src/lj_opt_narrow.c b/src/lj_opt_narrow.c index 6b6f20d3..6e3e9533 100644 --- a/src/lj_opt_narrow.c +++ b/src/lj_opt_narrow.c @@ -553,9 +553,9 @@ TRef lj_opt_narrow_unm(jit_State *J, TRef rc, TValue *vc) rc = conv_str_tonum(J, rc, vc); if (tref_isinteger(rc)) { uint32_t k = (uint32_t)numberVint(vc); - if ((LJ_DUALNUM || k != 0) && k != 0x80000000u) { + if ((tvisint(vc) || k != 0) && k != 0x80000000u) { TRef zero = lj_ir_kint(J, 0); - if (!LJ_DUALNUM) + if (!tvisint(vc)) emitir(IRTGI(IR_NE), rc, zero); return emitir(IRTGI(IR_SUBOV), zero, rc); } diff --git a/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua b/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua new file mode 100644 index 00000000..84f17953 --- /dev/null +++ b/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua @@ -0,0 +1,49 @@ +local tap = require('tap') + +-- This test demonstrates LuaJIT's incorrect narrowing +-- optimization in the DUALNUM mode for 0. +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1418. + +local test = tap.test('lj-1418-dualnum-narrowing-minus-0'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) +cannot reproduce an original bug with reverted fix. CMake configuration: CFLAGS=-DDUALNUM cmake -S . -B build -DCMAKE_BUILD_TYPE=DebugLuaJIT should be configured like: | cmake -DLUAJIT_NUMMODE=2 # ... <snipped>+-- Reset hotcounts. +jit.opt.start('hotloop=1') + +-- Hot trace. +test_non_const_on_trace(2, 3) +-- Record trace, use non zero result value to record.s/non zero/non-zero/Fixed, branch is force-pushed: =================================================================== diff --git a/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua b/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua index 84f17953..8f4185ef 100644 --- a/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua +++ b/test/tarantool-tests/lj-1418-dualnum-narrowing-minus-0.test.lua @@ -41,7 +41,7 @@ jit.opt.start('hotloop=1') -- Hot trace. test_non_const_on_trace(2, 3) --- Record trace, use non zero result value to record. +-- Record trace, use non-zero result value to record. test_non_const_on_trace(2, 3) -- Misbehaviour on trace with result zero value. test:is(test_non_const_on_trace(2, 1), '-0', 'correct non-const value on trace') ===================================================================+test_non_const_on_trace(2, 3) +-- Misbehaviour on trace with result zero value. +test:is(test_non_const_on_trace(2, 1), '-0', 'correct non-const value on trace') + +test:done(true)