[foreign-memaccess+abi] RFR: 8295290: Add Windows ARM64 ABI support to the Foreign Function & Memory API [v2]

[Jorn Vernee]

On Sun, 1 Jan 2023 06:16:01 GMT, Saint Wesonga <duke at openjdk.org> wrote:

>> There are 2 primary differences between the Windows ARM64 ABI and the macOS/Linux ARM64 ABI: variadic floating point arguments are passed in general purpose registers on Windows (instead of the vector registers). In addition to this, up to 64 bytes of a struct being passed to a variadic function can be placed in general purpose registers. This happens regardless of the type of struct (HFA or other generic struct). This means that a struct can be split across registers and the stack when invoking a variadic function.
>> 
>> This change introduces tests that compute the sum of the fields of structs containing 1-4 ints, floats, and doubles to verify that each field is correctly assigned a register or stack location when invoking a variadic function (both when the struct can be passed entirely in registers as well as when the struct spills onto the stack).
>> 
>> For details about the Foreign Function & Memory API, see JEP 434 defined at https://openjdk.org/jeps/434
>> 
>> The Windows ARM64 ABI conventions are documented at https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions
>> 
>> This work builds on @lewurm / Bernhard's branch at https://github.com/lewurm/openjdk/commits/foreign-windows-aarch64
>
> Saint Wesonga has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge branch 'foreign-memaccess+abi' into WindowsAArch64ABI
>  - Update Windows FFM implementation to match latest Preview API
>  - Merge branch 'foreign-memaccess+abi' into WindowsAArch64ABI
>  - Simplify newly added tests in TestVarArgs
>  - Move storage decisions into StorageCalculator
>  - Remove toSessionImpl method
>  - Remove unnecessary null check
>  - Move Windows CallArranger tests into separate file
>  - Add Windows ARM64 ABI support to the Foreign Function & Memory API
>    
>    There are 2 primary differences between the Windows ARM64 ABI and
>    the macOS/Linux ARM64 ABI: variadic floating point arguments are passed
>    in general purpose registers on Windows (instead of the vector registers).
>    In addition to this, up to 64 bytes of a struct being passed to a
>    variadic function can be placed in general purpose registers. This
>    happens regardless of the type of struct (HFA or other generic struct).
>    This means that a struct can be split across registers and the stack
>    when invoking a variadic function.
>    
>    This change introduces tests that compute the sum of the fields of
>    structs containing 1-4 ints, floats, and doubles to verify that each
>    field is correctly assigned a register or stack location when invoking
>    a variadic function (both when the struct can be passed entirely in
>    registers as well as when the struct spills onto the stack).
>    
>    For details about the Foreign Function & Memory API, see JEP 434
>    defined at https://openjdk.org/jeps/434
>    
>    The Windows ARM64 ABI conventions are documented at
>    https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions

Few more comments inline.

FWIW, I think CallArranger can be cleaned up further, and I've been playing with a patch [1]. But I'll file a followup PR for that.

[1]: https://github.com/openjdk/panama-foreign/compare/pull/754/head...JornVernee:panama-foreign:Refector_CallArranger

src/java.base/share/classes/jdk/internal/foreign/abi/LinkerOptions.java line 86:

> 84:     public boolean isVariadicFunction() {
> 85:         FirstVariadicArg fva = getOption(FirstVariadicArg.class);
> 86:         return fva != null && fva.index >= 0;

The second condition is not needed anymore, since we now validate that the index is greater than 0 already. (See `FirstVariadicArg::validateForDowncall`)

src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java line 263:

> 261: 
> 262:         VMStorage[] regAlloc(int type, MemoryLayout layout) {
> 263:             boolean spillRegistersPartially = forVariadicFunction && spillsVariadicStructsPartially();

Just be sure here, this would make it so that any struct passed to a variadic function is spilled partially, even if that particular struct is not being passed as a variadic argument.

Is that correct?

src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java line 287:

> 285:             if (type == StorageType.VECTOR) {
> 286:                 boolean forVariadicFunctionArgs = forArguments && forVariadicFunction;
> 287:                 boolean useIntRegsForFloatingPointArgs = forVariadicFunctionArgs && useIntRegsForVariadicFloatingPointArgs();

Same here I suppose. On Windows/x64 we pass vardiadic floats both in float registers and int registers, but only variadic floats. But, this seems to make it so that any float being passed to a variadic function, even as fixed argument, is passed in an int register.

test/jdk/java/foreign/valist/VaListTest.java line 136:

> 134:     private static final Function<Consumer<VaList.Builder>, VaList> linuxAArch64VaListFactory
> 135:             = actions -> LinuxAArch64Linker.newVaList(actions, SegmentScope.auto());
> 136:             private static final Function<Consumer<VaList.Builder>, VaList> macAArch64VaListFactory

Spurious whitespace
Suggestion:

    private static final Function<Consumer<VaList.Builder>, VaList> macAArch64VaListFactory

-------------

PR: https://git.openjdk.org/panama-foreign/pull/754