<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hello, Sergey,</p>
<p>thanks for the patch! Please see my comments.<br>
</p>
<p>Sergey<br>
</p>
<div class="moz-cite-prefix">On 5/14/25 14:56, Sergey Kaplun wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20250514115656.13243-1-skaplun@tarantool.org">
<pre wrap="" class="moz-quote-pre">From: Mike Pall <mike>
Reported by Gate88.
(cherry picked from commit 6c4826f12c4d33b8b978004bc681eb1eef2be977)
The issue is in the case when IR SLOAD is unused on a trace, persists</pre>
</blockquote>
typo: s/, /, it/<br>
<blockquote type="cite"
cite="mid:20250514115656.13243-1-skaplun@tarantool.org">
<pre wrap="" class="moz-quote-pre">
only for typecheck, and has the `num` type. In this case, the `dest`
register is `RID_NONE`. Hence, the `fmov` instruction is emitted</pre>
</blockquote>
<p>there are two instructions `fmov` in the generated assembler
output, which one do you mean?</p>
<p>55690afedc fmov d0, x30<br>
55690afee0 ldr w28, [x19, #8]<br>
55690afee4 ldur x3, [x19, #-16]<br>
55690afee8 and x3, x3, #0x7fffffffffff<br>
55690afeec ldr x27, [x3, #48]<br>
55690afef0 ldr x8, [x27, #32]<br>
55690afef4 sub x9, x8, x19<br>
55690afef8 cmp x9, #56<br>
55690afefc bls 0x690affe8 ->0<br>
55690aff00 ldr x6, [x8]<br>
55690aff04 asr x27, x6, #47<br>
55690aff08 cmn x27, #12<br>
55690aff0c bne 0x690affe8 ->0<br>
55690aff10 and x6, x6, #0x7fffffffffff<br>
55690aff14 ldr w7, [x6, #52]<br>
55690aff18 cmp w7, #1<br>
55690aff1c bne 0x690affe8 ->0<br>
55690aff20 ldr x5, [x6, #40]<br>
55690aff24 ldr x27, [x5, #32]<br>
55690aff28 cmp x27, x1<br>
55690aff2c bne 0x690affe8 ->0<br>
55690aff30 ldr x27, [x5, #24]<br>
55690aff34 cmp x0, x27, lsr #32<br>
55690aff38 bls 0x690affe8 ->0<br>
55690aff3c fmov d2, x27</p>
<p>or (probably) you just refer to a C code of `asm_sload`, that
emits `fmov`:</p>
<p> 1180 if (ra_hasreg(dest) && irt_isnum(t) &&
!(ir->op2 & IRSLOAD_CONVERT)) <br>
1181 emit_dn(as, A64I_FMOV_D_R, (dest & 31), tmp); <br>
</p>
<p>Sorry, it is not easy for me to match IR and ASM, so I believe
more details is required in the description.</p>
<p>I just want to understand a difference for emitted assembler
before and after the patch.<br>
</p>
<blockquote type="cite"
cite="mid:20250514115656.13243-1-skaplun@tarantool.org">
<pre wrap="" class="moz-quote-pre">
unconditionally, where the destination register is `d0` (`RID_NONE &
31`). So, the value of this register is spoiled. If it holds any value
evaluated before and used after this SLOAD, it leads to incorrect
behaviour.
This patch adds the check that the register is in use before emitting
the instruction.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#11278
---
Branch: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/skaplun/lj-903-arm64-unused-number-sload-typecheck">https://github.com/tarantool/luajit/tree/skaplun/lj-903-arm64-unused-number-sload-typecheck</a>
Related issues:
* <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/903">https://github.com/LuaJIT/LuaJIT/issues/903</a>
* <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/11278">https://github.com/tarantool/tarantool/issues/11278</a>
src/lj_asm_arm64.h | 2 +-
...m64-unused-number-sload-typecheck.test.lua | 45 +++++++++++++++++++
2 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua
diff --git a/src/lj_asm_arm64.h b/src/lj_asm_arm64.h
index 554bb60a..9b27473c 100644
--- a/src/lj_asm_arm64.h
+++ b/src/lj_asm_arm64.h
@@ -1177,7 +1177,7 @@ dotypecheck:
tmp = ra_scratch(as, allow);
rset_clear(allow, tmp);
}
- if (irt_isnum(t) && !(ir->op2 & IRSLOAD_CONVERT))
+ if (ra_hasreg(dest) && irt_isnum(t) && !(ir->op2 & IRSLOAD_CONVERT))
emit_dn(as, A64I_FMOV_D_R, (dest & 31), tmp);
/* Need type check, even if the load result is unused. */
asm_guardcc(as, irt_isnum(t) ? CC_LS : CC_NE);
diff --git a/test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua b/test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua
new file mode 100644
index 00000000..748b88e2
--- /dev/null
+++ b/test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua
@@ -0,0 +1,45 @@
+local tap = require('tap')
+-- Test file to demonstrate the incorrect JIT assembling of unused
+-- `IR_SLOAD` with number type on arm64.
+-- See also <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issue/903">https://github.com/LuaJIT/LuaJIT/issue/903</a>.</pre>
</blockquote>
typo: s/issue/issues/<br>
<blockquote type="cite"
cite="mid:20250514115656.13243-1-skaplun@tarantool.org">
<pre wrap="" class="moz-quote-pre">
+local test = tap.test('lj-903-arm64-unused-number-sload-typecheck'):skipcond({
+ ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- Just use any different numbers (but not integers to avoid
+-- integer IR type).
+local SLOT = 0.1
+local MARKER_VALUE = 4.2
+-- XXX: Special mapping to avoid folding and removing always true
+-- comparison.
+local anchor = {marker = MARKER_VALUE}
+
+-- Special function to inline on trace to generate SLOAD
+-- typecheck.
+local function sload_unused(x)
+ return x
+end
+
+-- The additional wrapper to use stackslots in the function.
+local function test_sload()
+ local sload = SLOT
+ for _ = 1, 4 do
+ -- This line should use the `d0` register.
+ local marker = anchor.marker - MARKER_VALUE
+ -- This generates unused IR_SLOAD with typecheck (number).
+ -- Before the patch, it occasionally overwrites the `d0`
+ -- register and causes the execution of the branch.
+ sload_unused(sload)
+ if marker ~= 0 then
+ return false
+ end
+ end
+ return true
+end
+
+jit.opt.start('hotloop=1')
+test:ok(test_sload(), 'correct SLOAD assembling')
+
+test:done(true)
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>