Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] x86/x64: Don't use undefined MUL/IMUL zero flag.
@ 2025-08-20 11:49 Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; only message in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-08-20 11:49 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

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


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-08-20 11:49 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-20 11:49 [Tarantool-patches] [PATCH luajit] x86/x64: Don't use undefined MUL/IMUL zero flag Sergey Kaplun via Tarantool-patches

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