[Tarantool-patches] [PATCH luajit 3/5] ARM64: Fix pass-by-value struct calling conventions.
Sergey Kaplun
skaplun at tarantool.org
Thu Jun 4 13:05:52 MSK 2026
Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.
On 01.06.26, Sergey Bronnikov wrote:
> Hi, Sergey,
>
> thanks for the patch! Please see my comments.
>
> Sergey
>
> On 5/30/26 19:04, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
<snipped>
> > ---
> > src/lj_ccall.c | 18 +++++++++++----
> > test/tarantool-tests/ffi-ccall/CMakeLists.txt | 1 +
> > test/tarantool-tests/ffi-ccall/libfficcall.c | 12 ++++++++++
> > ...57-arm64-struct-array-pass-by-val.test.lua | 23 +++++++++++++++++++
> > 4 files changed, 49 insertions(+), 5 deletions(-)
> > create mode 100644 test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua
> >
> > diff --git a/src/lj_ccall.c b/src/lj_ccall.c
> > index b2705de5..104c9d34 100644
> > --- a/src/lj_ccall.c
> > +++ b/src/lj_ccall.c
> > @@ -781,17 +781,24 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct)
> > {
> > CTSize sz = ct->size;
> > unsigned int r = 0, n = 0, isu = (ct->info & CTF_UNION);
> > - while (ct->sib) {
> > + while (ct->sib && n <= 4) {
>
> The patch adds a condition that strictly checks a number of elements
> with the same type (n <= 4).
It just early return as for condition below.
>
> I would also add a test for this with the following `n`: 3/4/5.
The n==3 is already checked in our tests. Do we need to test 5
specifically here? There are the similar tests for n > 4 in the next
commits as well.
>
> > + unsigned int m = 1;
> > CType *sct;
> > ct = ctype_get(cts, ct->sib);
> > if (ctype_isfield(ct->info)) {
> > sct = ctype_rawchild(cts, ct);
> > + if (ctype_isarray(sct->info)) {
> > + CType *cct = ctype_rawchild(cts, sct);
> > + if (!cct->size) continue;
> > + m = sct->size / cct->size;
> > + sct = cct;
> > + }
> > if (ctype_isfp(sct->info)) {
> > r |= sct->size;
> > - if (!isu) n++; else if (n == 0) n = 1;
> > + if (!isu) n += m; else if (n < m) n = m;
>
> The patch also touches a logic for unions (here and below), and it is
> desired to test it as well.
Added the test for unions.
===================================================================
diff --git a/test/tarantool-tests/ffi-ccall/libfficcall.c b/test/tarantool-tests/ffi-ccall/libfficcall.c
index 60d30f3c..2145b556 100644
--- a/test/tarantool-tests/ffi-ccall/libfficcall.c
+++ b/test/tarantool-tests/ffi-ccall/libfficcall.c
@@ -94,6 +94,10 @@ typedef struct hfa_float2 {
float v[2];
} hfa_float2;
+typedef union uhfa_float2 {
+ float v[2];
+} uhfa_float2;
+
typedef struct hfa_float22 {
float v[2][2];
} hfa_float22;
@@ -119,6 +123,11 @@ float hfa_float2_sum(hfa_float2 h)
return h.v[0] + h.v[1];
}
+float uhfa_float2_sum(uhfa_float2 h)
+{
+ return h.v[0] + h.v[1];
+}
+
float hfa_float22_sum(hfa_float22 h)
{
return h.v[0][0] + h.v[0][1] + h.v[1][0] + h.v[1][1];
diff --git a/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua b/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua
index bb500de1..b67533cf 100644
--- a/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua
+++ b/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua
@@ -6,7 +6,7 @@ local tap = require('tap')
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1357.
local test = tap.test('lj-1357-arm64-struct-array-pass-by-val')
-test:plan(1)
+test:plan(2)
local ffi_ccall = ffi.load('libfficcall')
@@ -16,8 +16,15 @@ ffi.cdef[[
} hfa_float2;
float hfa_float2_sum(hfa_float2 h);
+
+ typedef union uhfa_float2 {
+ float v[2];
+ } uhfa_float2;
+
+ float uhfa_float2_sum(uhfa_float2 h);
]]
test:is(ffi_ccall.hfa_float2_sum({{1, 2}}), 3, 'HFA float correct')
+test:is(ffi_ccall.uhfa_float2_sum({{1, 2}}), 3, 'union HFA float correct')
test:done(true)
===================================================================
>
> This change was not caught by our regression tests on Apple M2:
>
> --- a/src/lj_ccall.c
> +++ b/src/lj_ccall.c
> @@ -742,7 +742,7 @@ static unsigned int ccall_classify_struct(CTState
> *cts, CType *ct, CType *ctf)
> sct = ctype_rawchild(cts, ct);
> if (ctype_isfp(sct->info)) {
> r |= sct->size;
> - if (!isu) n++; else if (n == 0) n = 1;
> + if (!isu) n--; else if (n == 0) n = 1;
This is weird, since it changes non-union behaviour.
> } else if (ctype_iscomplex(sct->info)) {
> r |= (sct->size >> 1);
> if (!isu) n += 2; else if (n < 2) n = 2;
>
<snipped>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list