<!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>Thanks! LGTM<br>
</p>
<div class="moz-cite-prefix">On 6/30/25 10:26, Sergey Kaplun wrote:<br>
</div>
<blockquote type="cite" cite="mid:aGI8IIVX8HfKci7G@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the review!
Fixed your comments and force-pushed the branch.
On 27.06.25, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">Hi, Sergey,
thanks for the patch! LGTM with a minor two comments below.
Sergey
On 6/26/25 18:12, Sergey Kaplun wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">From: Mike Pall <mike>
Thanks to Peter Cawley.
(cherry picked from commit 0fa2f1cbcf023ad0549f1428809e506fa2c78552)
The arm64 emitting of load/store operation works incorrectly in the
case when at least one offset of load/store to be fused into ldp/stp is
misaligned. In this case this misaligning is ignored, and instructions
are fused, which leads to loading/storing from/to at least one incorrect
address.
For example, the following instructions:
| stur w0, [x1, #17]
| stur w0, [x1, #21]
May be fused to the following:
| stp w0, w0, [x1, #16]
This patch prevents fusion in this case by testing the alignment with
the help of bitwise ROR by the alignment value. In case of misaligned
offset, the value overflows the 7-bit length mask in the check.
The negative immediate (7-bit width including sign bit) is limited by
the corresponding addition of `64 << sc` (it is harmless in the case of
positive values).
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#11278
---
Related issues:
*<a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1056">https://github.com/LuaJIT/LuaJIT/issues/1056</a>
*<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/11278">https://github.com/tarantool/tarantool/issues/11278</a>
<a class="moz-txt-link-freetext" href="Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1056-arm64-ldp-sdp-misaligned-fusing">Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1056-arm64-ldp-sdp-misaligned-fusing</a>
src/lj_emit_arm64.h | 2 +-
...6-arm64-ldp-sdp-misaligned-fusing.test.lua | 98 +++++++++++++++++++
2 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
diff --git a/src/lj_emit_arm64.h b/src/lj_emit_arm64.h
index 30cd3505..5c1bc372 100644
--- a/src/lj_emit_arm64.h
+++ b/src/lj_emit_arm64.h
</pre>
</blockquote>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua b/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
new file mode 100644
index 00000000..5d03097e
--- /dev/null
+++ b/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
@@ -0,0 +1,98 @@
+local tap = require('tap')
+local ffi = require('ffi')
+
+-- This test demonstrates LuaJIT's incorrect emitting of LDP/STP
+-- instructions from LDUR/STUR instructions with misaligned offset
+-- on arm64.
+-- See <a class="moz-txt-link-freetext" href="alsohttps://github.com/LuaJIT/LuaJIT/issue/1056">alsohttps://github.com/LuaJIT/LuaJIT/issue/1056</a>.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">s/issue/issues/
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Fixed, thanks!
===================================================================
diff --git a/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua b/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
index 815da15d..5ff040e7 100644
--- a/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
+++ b/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
@@ -4,7 +4,7 @@ local ffi = require('ffi')
-- This test demonstrates LuaJIT's incorrect emitting of LDP/STP
-- instructions from LDUR/STUR instructions with misaligned offset
-- on arm64.
--- See also <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issue/1056">https://github.com/LuaJIT/LuaJIT/issue/1056</a>.
+-- See also <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1056">https://github.com/LuaJIT/LuaJIT/issues/1056</a>.
local test = tap.test('lj-1056-arm64-ldp-sdp-misaligned-fusing'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
})
===================================================================
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+local test = tap.test('lj-1056-arm64-ldp-sdp-misaligned-fusing'):skipcond({
+ ['Test requires JIT enabled'] = not jit.status(),
+})
+
</pre>
</blockquote>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+
+test:is(resl, 0x4030201, 'pair of misaligned loads, left')
+test:is(resr, 0x8070605, 'pair of misaligned loads, right')
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
What does mean these magic numbers? Please add a comment or
use a variable with self-explained name. Here and below.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Added the comments nearby, see the corresponding patch below:
===================================================================
diff --git a/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua b/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
index 5d03097e..815da15d 100644
--- a/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
+++ b/test/tarantool-tests/lj-1056-arm64-ldp-sdp-misaligned-fusing.test.lua
@@ -81,6 +81,8 @@ for _ = 1, N_ITERATIONS do
resr = ffi.cast('int32_t *', ptr + 5)[0]
end
+-- Values are resulted from the `init_buf()` function with the
+-- corresponding offset.
<a class="moz-txt-link-freetext" href="test:is(resl">test:is(resl</a>, 0x4030201, 'pair of misaligned loads, left')
<a class="moz-txt-link-freetext" href="test:is(resr">test:is(resr</a>, 0x8070605, 'pair of misaligned loads, right')
@@ -92,6 +94,8 @@ for _ = 1, N_ITERATIONS do
resl = ffi.cast('int32_t *', ptr)[0]
end
+-- Values are resulted from the `init_buf()` function with the
+-- corresponding offset.
<a class="moz-txt-link-freetext" href="test:is(resl">test:is(resl</a>, 0x3020100, 'aligned / misaligned load, aligned')
<a class="moz-txt-link-freetext" href="test:is(resr">test:is(resr</a>, 0x8070605, 'aligned / misaligned load, misaligned')
===================================================================
<snipped>
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>