From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id EBA9B6EFE5; Thu, 4 Jun 2026 13:06:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EBA9B6EFE5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1780567590; bh=jQhvOtyiJiTkY02USdL9gTGgI5zUsCqTBouBPceuxDg=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=tnNR+n4yMPa9e3Bm8CN9B7BpSTaQf49qr0gqSFRfQrElxcNhfOkAldRgQjtq2tAuM /Z0JrQ8bTlOI6AcGbnpi/JhtIyMog++hfeB4PVT/OSzh8C11BGv64XzsEztKgyjvwo CWpOJUxhuBlA4tHDu9407WcYccrKjHVm8SUsW1c8= Received: from send104.i.mail.ru (send104.i.mail.ru [89.221.237.199]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C5F906EFE5 for ; Thu, 4 Jun 2026 13:06:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C5F906EFE5 Received: by exim-smtp-5b85998476-g9x28 with esmtpa (envelope-from ) id 1wV4y3-0000000008M-2jHj; Thu, 04 Jun 2026 13:06:28 +0300 Date: Thu, 4 Jun 2026 13:05:52 +0300 To: Sergey Bronnikov Message-ID: References: <20260530160409.4043089-1-skaplun@tarantool.org> <20260530160409.4043089-4-skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9FAD06046D747065BBF816D40BB268C394B27FCF4C446E11A182A05F5380850402C6CC983A611CCD43DE06ABAFEAF6705B28E2368FB2B816C22FB8A326B5113B66C140D91676DDD80 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE705C173BDDADFC82AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB5533756660A56BB7F6F20B609647E54EA9B3766223777CD217F3EB0FE64B136242DFA127389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0A3E989B1926288338941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6F459A8243F1D1D44CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE778B471BB9634AD8A731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A55426F3C41417A4985002B1117B3ED696FC353872855C4587108A05421C070DB8823CB91A9FED034534781492E4B8EEAD4ECBDE8281F904F9BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE191716CD42B3DD1D34CAB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D6590B391D9F9957F6D67C329AF264644BC6F657DE5A5538DC90D8080B945AF36CE877F2DD83A34E24C0B8341EE9D5BE9A0A1616A4102B8A19DD3BA3F8B02A6D222B733AE90B19F2BFA06536EB022892E5344C41F94D744909CECFA6C6B0C050A61A8CAF69B82BA93681CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVRI2994ruhLUMXFFU7xOUPk= X-DA7885C5: 64E06E32B1CBACCBF255D290C0D534F972F2926C3E00E2E373C31AD06928C98E56526170FC2243685B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393520AF17B8A65FDE23E2F9B44957A0EA41BA244448155491D94A2104D7755E442E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/5] ARM64: Fix pass-by-value struct calling conventions. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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 > > --- > > 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; > -- Best regards, Sergey Kaplun