Re: [Tarantool-patches] [PATCH luajit 4/5] FFI: Various ABI and calling convention fixes.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey,
Thanks for the review!
Fixed your comments and force-pushed the branch.

On 01.06.26, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! See my comments below.
> 
> It looks like you tried to test the patch as thoroughly as possible.
> 
> Could you tell me how you compiled your test list?

Some ideas is taken from [1][2][3].

> 
> Is there a matrix of possible types and structures you used?

I just use various tests related to the changes in the patch. Several
packed structures. Structures of different sizes. Several aligned
structures. All of it to be passed through the stack and/or registers.
HFA aggregates. Structure with 0-bitfields.

>                                                              How do you 
> assess the completeness of testing?

The complete coverage of all C calling conventions is not the goal of
this patch.

> 
> Sergey
> 
> On 5/30/26 19:04, Sergey Kaplun wrote:

<snipped>

> > diff --git a/test/tarantool-tests/ffi-ccall/CMakeLists.txt b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
> > index 1d004591..dfd58bd2 100644
> > --- a/test/tarantool-tests/ffi-ccall/CMakeLists.txt
> > +++ b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
> > @@ -1,8 +1,12 @@
> >   list(APPEND tests
> > +  ffi-call-empty-struct.test.lua
> > +  ffi-vector-arguments.test.lua
> >     ffi-ccall-arm64-fp-convention.test.lua
> >     lj-205-arm64-osx-ffi-enum-arg.test.lua
> >     lj-205-arm64-osx-ffi-small-arg.test.lua
> >     lj-1357-arm64-struct-array-pass-by-val.test.lua
> > +  lj-1455-arm64-ffi-ccall-hfa.test.lua
> > +  lj-1455-ffi-conventions.test.lua
> >   )
> 
> it is sorted not alphabetically:

Fixed.

> 
> --- a/test/tarantool-tests/ffi-ccall/CMakeLists.txt
> +++ b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
> @@ -1,12 +1,12 @@
>   list(APPEND tests
>     ffi-call-empty-struct.test.lua
> -  ffi-vector-arguments.test.lua
>     ffi-ccall-arm64-fp-convention.test.lua
> -  lj-205-arm64-osx-ffi-enum-arg.test.lua
> -  lj-205-arm64-osx-ffi-small-arg.test.lua
> +  ffi-vector-arguments.test.lua
>     lj-1357-arm64-struct-array-pass-by-val.test.lua
>     lj-1455-arm64-ffi-ccall-hfa.test.lua
>     lj-1455-ffi-conventions.test.lua
> +  lj-205-arm64-osx-ffi-enum-arg.test.lua
> +  lj-205-arm64-osx-ffi-small-arg.test.lua

In order of common sense, I leave the numeric sorting order for ticket
numbers:

===================================================================
diff --git a/test/tarantool-tests/ffi-ccall/CMakeLists.txt b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
index dfd58bd2..6bc839c3 100644
--- a/test/tarantool-tests/ffi-ccall/CMakeLists.txt
+++ b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
@@ -1,7 +1,7 @@
 list(APPEND tests
   ffi-call-empty-struct.test.lua
-  ffi-vector-arguments.test.lua
   ffi-ccall-arm64-fp-convention.test.lua
+  ffi-vector-arguments.test.lua
   lj-205-arm64-osx-ffi-enum-arg.test.lua
   lj-205-arm64-osx-ffi-small-arg.test.lua
   lj-1357-arm64-struct-array-pass-by-val.test.lua
===================================================================

>   )
> 
> 

<snipped>

> > diff --git a/test/tarantool-tests/ffi-vector-arguments.test.lua b/test/tarantool-tests/ffi-vector-arguments.test.lua
> > new file mode 100644
> > index 00000000..330ec991
> > --- /dev/null
> > +++ b/test/tarantool-tests/ffi-vector-arguments.test.lua
> > @@ -0,0 +1,62 @@
> > +local ffi = require('ffi')
> > +local tap = require('tap')
> > +
> > +-- The test file to check FFI vector passing correctness.
> > +local test = tap.test('ffi-vector-arguments'):skipcond({
> > +  ['NYI for non x64 arches'] = jit.arch ~= 'x64',
> s/non x64/non-x64/

Fixed.
===================================================================
diff --git a/test/tarantool-tests/ffi-vector-arguments.test.lua b/test/tarantool-tests/ffi-vector-arguments.test.lua
index 555e7fab..42083487 100644
--- a/test/tarantool-tests/ffi-vector-arguments.test.lua
+++ b/test/tarantool-tests/ffi-vector-arguments.test.lua
@@ -3,7 +3,7 @@ local tap = require('tap')
 
 -- The test file to check FFI vector passing correctness.
 local test = tap.test('ffi-vector-arguments'):skipcond({
-  ['NYI for non x64 arches'] = jit.arch ~= 'x64',
+  ['NYI for non-x64 arches'] = jit.arch ~= 'x64',
 })
 
 local SIZING
===================================================================

> > +})
> > +
> > +local SIZING
> > +-- Only those are implemented.
> > +if jit.arch == 'x64' then
> > +  SIZING = {2, 4,}
> s/,}/}/

Fixed.
===================================================================
diff --git a/test/tarantool-tests/ffi-vector-arguments.test.lua b/test/tarantool-tests/ffi-vector-arguments.test.lua
index 330ec991..555e7fab 100644
--- a/test/tarantool-tests/ffi-vector-arguments.test.lua
+++ b/test/tarantool-tests/ffi-vector-arguments.test.lua
@@ -9,7 +9,7 @@ local test = tap.test('ffi-vector-arguments'):skipcond({
 local SIZING
 -- Only those are implemented.
 if jit.arch == 'x64' then
-  SIZING = {2, 4,}
+  SIZING = {2, 4}
 else
   SIZING = {}
 end
===================================================================

> > +else
> > +  SIZING = {}

<snipped>

[1]: https://gnu.googlesource.com/gcc/+/8fcc61f8964aa9aa2e6fc08cb961f9dc2a5add77/gcc/testsuite/gcc.target/aarch64/aapcs64
[2]: https://github.com/llvm/llvm-project/tree/main/clang/test/CodeGen/AArch64
[3]: https://github.com/llvm/llvm-project/tree/main/clang/test

-- 
Best regards,
Sergey Kaplun