Re: [Tarantool-patches] [PATCH luajit] x86/x64: Don't use undefined MUL/IMUL zero flag.

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

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

Hi,  Sergey,

thanks for the patch! LGTM

Sergey

On 8/20/25 14:49, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by VrIgHtEr.
>
> (cherry picked from commit c92d0cb19263e7e302b4740ba6617a32c201c613)
>
> When emitting the arithmetic operations on registers via the
> `asm_intarith()`, the next `test` instruction may be dropped since the
> flag register is modified by the arithmetic instruction to be emitted.
>
> But the `imul` instruction [1] doesn't modify ZF, so its value is
> undefined. This patch prevents dropping the `test` instruction if
> the emitted instruction is `imul`.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> [1]:https://www.felixcloutier.com/x86/imul
>
> Part of tarantool/tarantool#11691
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1376-undefined-mul-test-flag
> Related issues:
> *https://github.com/tarantool/tarantool/issues/11691
> *https://github.com/LuaJIT/LuaJIT/issues/1376
>
>   src/lj_asm_x86.h                              |  3 +-
>   .../lj-1376-undefined-mul-test-flag.test.lua  | 38 +++++++++++++++++++
>   2 files changed, 40 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/lj-1376-undefined-mul-test-flag.test.lua
>
> diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h
> index 89e83205..0e3a473c 100644
> --- a/src/lj_asm_x86.h
> +++ b/src/lj_asm_x86.h
> @@ -2061,7 +2061,8 @@ static void asm_intarith(ASMState *as, IRIns *ir, x86Arith xa)
>     RegSet allow = RSET_GPR;
>     Reg dest, right;
>     int32_t k = 0;
> -  if (as->flagmcp == as->mcp) {  /* Drop test r,r instruction. */
> +  if (as->flagmcp == as->mcp && xa != XOg_X_IMUL) {
> +    /* Drop test r,r instruction. */
>       MCode *p = as->mcp + ((LJ_64 && *as->mcp < XI_TESTb) ? 3 : 2);
>       MCode *q = p[0] == 0x0f ? p+1 : p;
>       if ((*q & 15) < 14) {
> diff --git a/test/tarantool-tests/lj-1376-undefined-mul-test-flag.test.lua b/test/tarantool-tests/lj-1376-undefined-mul-test-flag.test.lua
> new file mode 100644
> index 00000000..f6c02a00
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1376-undefined-mul-test-flag.test.lua
> @@ -0,0 +1,38 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate incorrect assembling optimization
> +-- for x86/x64 CPUs.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1376.
> +
> +local test = tap.test('lj-1376-undefined-mul-test-flag'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(1)
> +
> +local a, b = 0ULL, 0ULL
> +
> +jit.opt.start('hotloop=1')
> +for _ = 1, 4 do
> +  -- Before the patch, the `test` instruction is dropped by
> +  -- assuming the `imul` instruction before it modifies the flags
> +  -- register. It results in the following mcode:
> +  -- | imul r15, rbp
> +  -- | jnz 0x559415b10060        ->5
> +  -- Instead of the following:
> +  -- | imul r15, rbp
> +  -- | test r15, r15
> +  -- | jnz 0x559415b10060        ->5
> +  -- This leads to the incorrect branch being taken.
> +  if a * b ~= 0ULL then
> +test:fail('the impossible branch is taken')
> +test:done(true)
> +  end
> +  -- XXX: Need to update multiplier to stay in the variant part of
> +  -- the loop, since invariant contains IR_NOP (former unused
> +  -- IR_CNEW) between IRs, and the optimization is not applied.
> +  b = b + 1
> +end
> +
> +test:ok(true, 'no dropping of test instruction')
> +test:done(true)

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