RFR: 8262901: [macos_aarch64] NativeCallTest expected:<-3.8194101E18> but was:<3.02668882E10> [v2]

[Tom Shull]

On Thu, 2 Dec 2021 07:00:52 GMT, Danil Bubnov <duke at openjdk.java.net> wrote:

>> This is the fix of aarch64 jvmci calling convention.
>> 
>> On MacOS/aarch64 "Function arguments may consume slots on the stack that are not multiples of 8 bytes" [1], but current approach uses only wordsize or bigger slots, which is incorrect (that is why tests were failing [4]). Now arguments consume the right amount of bytes.
>> 
>> Another problem is that current approach don't make 16-byte alignment of Stack Pointer [1][2][3]. However, tests not fail on Linux/aarch64 and Windows/aarch64. They pass because in this tests all functions have even number of argumets, that is why 16-byte alignment comes automatically. But if you try to add or delete one argumets, tests will fail with SIGBUS.
>> 
>> I've tested this patch on MacOS/aarch64 and Linux/aarch64, all tests have passed.
>> 
>> Also I don't understand, why current tests (NativeCallTest) use only int, long, float and double as arguments types. Is it possible to add functions with another types like byte or short? I tried, but it fails on every platform.
>> 
>> [1] https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms
>> [2] https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#the-stack
>> [3] https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-160#stack
>> [4] https://bugs.openjdk.java.net/browse/JDK-8262901
>
> Danil Bubnov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add test with an odd number of arguments

Changes requested by teshull at github.com (no known OpenJDK username).

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java line 272:

> 270:                 ValueKind<?> valueKind = valueKindFactory.getValueKind(kind);
> 271:                 int kindSize = valueKind.getPlatformKind().getSizeInBytes();
> 272:                 if (macOS && currentStackOffset % kindSize != 0) {

Shouldn't this logic only be included for HotSpotCallingConventionType.NativeCall, but not JavaCall/JavaCallee?

In native-image, the equivalent logic is here: https://github.com/oracle/graal/blob/cf81d2a7c3cbaa2574d0ecbbc2e76879b7719b20/substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64RegisterConfig.java#L333

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java line 287:

> 285:             }
> 286:         }
> 287:         if (currentStackOffset % target.stackAlignment != 0) {

I think this must be taken care of somewhere else. Otherwise, amd64 would also have this same problem, as its stack alignment is also 16 bytes

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

PR: https://git.openjdk.java.net/jdk/pull/6641