Re: [Tarantool-patches] [PATCH luajit 2/3] DUALNUM: Fix narrowing of unary minus.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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=Debug

LuaJIT 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)

-- 
Best regards,
Sergey Kaplun