Re: [Tarantool-patches] [PATCH luajit] ARM64: Fix assembly of HREFK (again).

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

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

Hi, Sergey,

thanks for the patch! LGTM

Sergey

On 9/18/25 16:55, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry picked from commit 93ce12ee15abf28ef4cb24ae7e4b8a5b73d75c85)
>
> When assembling the HREFK IR with the huge offset of the target node
> from the table, this offset calculation and the key loading from the
> node are emitted like the following:
> |  ldr   x16, [x2, 40]
> |  add   x16, x16, x21
> |  ldr   x27, [x16, 8]
> |  cmp   x27, x17
>
> Here, `x16` is the node register, `x27` is the key register, and `x21`
> is the register containing the offset.
>
> It is possible that the register for holding the constant operand in the
> addition may be chosen as the same register containing the node address,
> since the full `RSET_GPR` is given to the `emit_opk()`. It will result
> in the following invalid mcode:
> |  ldr   x27, [x2, 40]
> |  str   x27, [sp, 8]
> |  add   x16, x16, x16
> |  ldr   x16, [sp, 8]
> |  ldr   x27, [x16, 8]
> |  cmp   x27, x17
>
> It seems that in the current implementation the LuaJIT's register
> allocator always prefers the register holding the key instead, so this
> does not lead to the invalid emitting. Hence, it is impossible to come
> up with any valid reproducer. However, to avoid possible regressions in
> the future, this patch fixes the invalid register set by excluding the
> node register from it.
>
> Sergey Kaplun:
> * added the description for the problem
>
> Part of tarantool/tarantool#11691
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1026-fix-ra-hrefk
> Related issues:
> *https://github.com/tarantool/tarantool/issues/11691
> *https://github.com/LuaJIT/LuaJIT/issues/1026
>
> The issue isn't reproduced even with the RANDOM_RA, so I suppose we may
> apply the patch without a test case.
>
>   src/lj_asm_arm64.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/lj_asm_arm64.h b/src/lj_asm_arm64.h
> index 9b3c0467..313b4a96 100644
> --- a/src/lj_asm_arm64.h
> +++ b/src/lj_asm_arm64.h
> @@ -911,7 +911,7 @@ static void asm_hrefk(ASMState *as, IRIns *ir)
>     emit_nm(as, A64I_CMPx, key, ra_allock(as, k, rset_exclude(allow, key)));
>     emit_lso(as, A64I_LDRx, key, idx, kofs);
>     if (bigofs)
> -    emit_opk(as, A64I_ADDx, dest, node, ofs, RSET_GPR);
> +    emit_opk(as, A64I_ADDx, dest, node, ofs, rset_exclude(RSET_GPR, node));
>   }
>   
>   static void asm_uref(ASMState *as, IRIns *ir)

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