Re: [Tarantool-patches] [PATCH luajit 1/1] x86/x64: Change ipairs_aux to match JIT backend behavior.

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

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

Hi, Sergey,

thanks for the patch! LGTM with a minor comment(s).

Sergey

On 6/5/26 15:49, Sergey Kaplun wrote:
> 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

s/Vanilla/PUC Rio/

feel free to ignore

> 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
>     |  movTAB: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)

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