Tarantool development patches archive
 help / color / mirror / Atom feed
From: Evgeniy Temirgaleev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: "Sergey Kaplun" <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches]  [PATCH luajit 1/1] x86/x64: Change ipairs_aux to match JIT backend behavior.
Date: Tue, 16 Jun 2026 01:49:21 +0300	[thread overview]
Message-ID: <1781563761.98986661@f554.i.mail.ru> (raw)
In-Reply-To: <20260605124909.2709341-1-skaplun@tarantool.org>

[-- Attachment #1: Type: text/plain, Size: 3833 bytes --]

Hi, Sergey!

Thanks for the patch!

LGTM

--
Best regards,
Evgeniy Temirgaleev

> 
> From: Sergey Kaplun <skaplun@tarantool.org>
> To: Sergey Bronnikov <sergeyb@tarantool.org>, Evgeniy Temirgaleev <e.temirgaleev@tarantool.org
> >
> Cc: tarantool-patches@dev.tarantool.org, Sergey Kaplun <skaplun@tarantool.org
> >
> Date: Friday, June 5, 2026 3:49 PM +03:00
> From: Mike Pall <mike>
> 
> Thanks to Sergey Kaplun.
> 
> (cherry picked from commit 64b1f10835acc18bf8923adf248dce4894867882)
> 
> In the single-number VM, `ipairs_aux()` first adds the 1 to the given
> number and only after casts it to an integer. This leads to results
> different from Vanilla Lua 5.1 and inconsistent with JIT engine
> recording.
> 
> This patch fixes it by casting the value to an integer before addition.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#12480
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1463-ipairs-aux-consistency
> 
> Related issues:
> * https://github.com/LuaJIT/LuaJIT/issues/1463
> * https://github.com/tarantool/tarantool/issues/12480
> 
> src/vm_x64.dasc | 8 ++---
> src/vm_x86.dasc | 7 ++--
> .../lj-1463-ipairs-aux-consistency.test.lua | 32 +++++++++++++++++++
> 3 files changed, 38 insertions(+), 9 deletions(-)
> create mode 100644
> test/tarantool-tests/lj-1463-ipairs-aux-consistency.test.lua
> 
> diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
> index 6ac88d70..1bd3c855 100644
> --- a/src/vm_x64.dasc
> +++ b/src/vm_x64.dasc
> @@ -1476,17 +1476,15 @@ static void build_subroutines(BuildCtx *ctx)
> | checkint RA, ->fff_fallback
> |.else
> | checknumtp [BASE+8], ->fff_fallback
> - | movsd xmm0, qword [BASE+8]
> + | cvttsd2si RAd, qword [BASE+8]
> |.endif
> | mov PC, [BASE-8]
> - |.if DUALNUM
> | add RAd, 1
> + |.if DUALNUM
> | setint ITYPE, RA
> | mov [BASE-16], ITYPE
> |.else
> - | sseconst_1 xmm1, TMPR
> - | addsd xmm0, xmm1
> - | cvttsd2si RAd, xmm0
> + | cvtsi2sd xmm0, RAd
> | movsd qword [BASE-16], xmm0
> |.endif
> | cmp RAd, TAB:RB->asize; jae >2 // Not in array part?
> diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
> index d9234f3b..dcfb0a8b 100644
> --- a/src/vm_x86.dasc
> +++ b/src/vm_x86.dasc
> @@ -1853,10 +1853,9 @@ static void build_subroutines(BuildCtx *ctx)
> | mov dword [BASE-4], LJ_TISNUM
> | mov dword [BASE-8], RD
> |.else
> - | movsd xmm0, qword [BASE+8]
> - | sseconst_1 xmm1, RBa
> - | addsd xmm0, xmm1
> - | cvttsd2si RD, xmm0
> + | cvttsd2si RD, qword [BASE+8]
> + | add RD, 1
> + | cvtsi2sd xmm0, RD
> | movsd qword [BASE-8], xmm0
> |.endif
> | mov TAB:RB, [BASE]
> diff --git a/test/tarantool-tests/lj-1463-ipairs-aux-consistency.test.lua
> b/test/tarantool-tests/lj-1463-ipairs-aux-consistency.test.lua
> new file mode 100644
> index 00000000..1da3a76e
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1463-ipairs-aux-consistency.test.lua
> @@ -0,0 +1,32 @@
> +local tap = require('tap')
> +
> +-- The test file to demonstrate the inconsistent behaviour between
> +-- the JIT compiler and the VM for the `ipairs_aux()` function on
> +-- x86 and x86_64 arches.
> +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1463.
> +
> +local test = tap.test('lj-1463-ipairs-aux-consistency'):skipcond({
> + ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(4)
> +
> +jit.opt.start('hotloop=1')
> +
> +local ipairs_aux = ipairs({})
> +
> +local rkeys = {}
> +local rvals = {}
> +
> +for i = 1, 4 do
> + local key, val = ipairs_aux({[0] = 0, [1] = 1}, -0.1)
> + rkeys[i] = key
> + rvals[i] = val
> +end
> +
> +test:is(rkeys[1], 1, 'correct key result')
> +test:is(rvals[1], 1, 'correct value result')
> +test:samevalues(rkeys, 'consistent JIT and VM behaviour for keys')
> +test:samevalues(rvals, 'consistent JIT and VM behaviour for values')
> +
> +test:done(true)
> --
> 2.54.0
>

[-- Attachment #2: Type: text/html, Size: 5073 bytes --]

      parent reply	other threads:[~2026-06-15 22:49 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-05 12:49 Sergey Kaplun via Tarantool-patches
2026-06-05 15:14 ` Sergey Bronnikov via Tarantool-patches
2026-06-05 15:47   ` Sergey Kaplun via Tarantool-patches
2026-06-15 22:49 ` Evgeniy Temirgaleev via Tarantool-patches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1781563761.98986661@f554.i.mail.ru \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=e.temirgaleev@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches]  [PATCH luajit 1/1] x86/x64: Change ipairs_aux to match JIT backend behavior.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox