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

Olga Mikhaltsova · AntonKozlov · commit 6b456f7a9b63 · 2022-11-10T21:06:15.000Z
Reviewed-by: aph
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.code/src/jdk/vm/ci/code/TargetDescription.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.code/src/jdk/vm/ci/code/TargetDescription.java
@@ -25,13 +25,17 @@
 import static jdk.vm.ci.meta.MetaUtil.identityHashCodeString;
 
 import jdk.vm.ci.meta.JavaKind;
+import jdk.vm.ci.services.Services;
 
 /**
  * Represents the target machine for a compiler, including the CPU architecture, the size of
  * pointers and references, alignment of stacks, caches, etc.
  */
 public class TargetDescription {
 
+    public final boolean linuxOs = Services.getSavedProperty("os.name", "").startsWith("Linux");
+    public final boolean macOs = Services.getSavedProperty("os.name", "").startsWith("Mac");
+
     public final Architecture arch;
 
     /**
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotJVMCIBackendFactory.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotJVMCIBackendFactory.java
@@ -93,7 +93,7 @@ private static RegisterConfig createRegisterConfig(AArch64HotSpotVMConfig config
         // ARMv8 defines r18 as being available to the platform ABI. Windows
         // and Darwin use it for such. Linux doesn't assign it and thus r18 can
         // be used as an additional register.
-        boolean canUsePlatformRegister = config.linuxOs;
+        boolean canUsePlatformRegister = target.linuxOs;
         return new AArch64HotSpotRegisterConfig(target, config.useCompressedOops, canUsePlatformRegister);
     }
 
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java
@@ -228,6 +228,27 @@ public RegisterArray getCallingConventionRegisters(Type type, JavaKind kind) {
         }
     }
 
+    private int parseStackArg(ValueKind<?> valueKind, AllocatableValue[] locations, int index, int currentStackOffset, HotSpotCallingConventionType type) {
+        int kindSize = valueKind.getPlatformKind().getSizeInBytes();
+        locations[index] = StackSlot.get(valueKind, currentStackOffset, !type.out);
+        currentStackOffset += Math.max(kindSize, target.wordSize);
+        return currentStackOffset;
+    }
+
+    private int parseDarwinNativeStackArg(ValueKind<?> valueKind, AllocatableValue[] locations, int index, int currentStackOffset, HotSpotCallingConventionType type) {
+        int kindSize = valueKind.getPlatformKind().getSizeInBytes();
+        if (currentStackOffset % kindSize != 0) {
+            // In MacOS natural alignment is used
+            // See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms
+            currentStackOffset += kindSize - currentStackOffset % kindSize;
+        }
+        locations[index] = StackSlot.get(valueKind, currentStackOffset, !type.out);
+        // In MacOS "Function arguments may consume slots on the stack that are not multiples of 8 bytes"
+        // See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms
+        currentStackOffset += kindSize;
+        return currentStackOffset;
+    }
+
     private CallingConvention callingConvention(RegisterArray generalParameterRegisters, JavaType returnType, JavaType[] parameterTypes, HotSpotCallingConventionType type,
                     ValueKindFactory<?> valueKindFactory) {
         AllocatableValue[] locations = new AllocatableValue[parameterTypes.length];
@@ -264,9 +285,11 @@ private CallingConvention callingConvention(RegisterArray generalParameterRegist
             }
 
             if (locations[i] == null) {
-                ValueKind<?> valueKind = valueKindFactory.getValueKind(kind);
-                locations[i] = StackSlot.get(valueKind, currentStackOffset, !type.out);
-                currentStackOffset += Math.max(valueKind.getPlatformKind().getSizeInBytes(), target.wordSize);
+                if (target.macOs && type == HotSpotCallingConventionType.NativeCall) {
+                    currentStackOffset = parseDarwinNativeStackArg(valueKindFactory.getValueKind(kind), locations, i, currentStackOffset, type);
+                } else {
+                    currentStackOffset = parseStackArg(valueKindFactory.getValueKind(kind), locations, i, currentStackOffset, type);
+                }
             }
         }
 
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotVMConfig.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotVMConfig.java
@@ -24,7 +24,6 @@
 
 import jdk.vm.ci.hotspot.HotSpotVMConfigAccess;
 import jdk.vm.ci.hotspot.HotSpotVMConfigStore;
-import jdk.vm.ci.services.Services;
 
 /**
  * Used to access native configuration details.
@@ -37,8 +36,6 @@ class AArch64HotSpotVMConfig extends HotSpotVMConfigAccess {
         super(config);
     }
 
-    final boolean linuxOs = Services.getSavedProperty("os.name", "").startsWith("Linux");
-
     final boolean useCompressedOops = getFlag("UseCompressedOops", Boolean.class);
 
     // CPU Capabilities
diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt
@@ -46,7 +46,6 @@
 compiler/ciReplay/TestSAServer.java 8029528 generic-all
 compiler/compilercontrol/jcmd/ClearDirectivesFileStackTest.java 8225370 generic-all
 compiler/jvmci/compilerToVM/GetFlagValueTest.java 8204459 generic-all
-compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/NativeCallTest.java 8262901 macosx-aarch64
 compiler/tiered/LevelTransitionTest.java 8067651 generic-all
 
 compiler/cpuflags/TestAESIntrinsicsOnSupportedConfig.java 8190680 generic-all
diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.code.test/libNativeCallTest.c b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.code.test/libNativeCallTest.c
@@ -189,6 +189,39 @@ JNIEXPORT jfloat JNICALL Java_jdk_vm_ci_code_test_NativeCallTest__1L32SDILDS(JNI
                    a,   b,   c,   d,   e,   f);
 }
 
+jint JNICALL I32I(jint i00, jint i01, jint i02, jint i03, jint i04, jint i05, jint i06, jint i07,
+                  jint i08, jint i09, jint i0a, jint i0b, jint i0c, jint i0d, jint i0e, jint i0f,
+                  jint i10, jint i11, jint i12, jint i13, jint i14, jint i15, jint i16, jint i17,
+                  jint i18, jint i19, jint i1a, jint i1b, jint i1c, jint i1d, jint i1e, jint i1f,
+                  jint a) {
+  return i00 + i01 + i02 + i03 + i04 + i05 + i06 + i07 +
+         i08 + i09 + i0a + i0b + i0c + i0d + i0e + i0f +
+         i10 + i11 + i12 + i13 + i14 + i15 + i16 + i17 +
+         i18 + i19 + i1a + i1b + i1c + i1d + i1e + i1f +
+         a;
+}
+
+JNIEXPORT jlong JNICALL Java_jdk_vm_ci_code_test_NativeCallTest_getI32I(JNIEnv *env, jclass clazz) {
+  return (jlong) (intptr_t) I32I;
+}
+
+JNIEXPORT jint JNICALL Java_jdk_vm_ci_code_test_NativeCallTest__1I32I(JNIEnv *env, jclass clazz,
+                                                                      jint i00, jint i01, jint i02, jint i03,
+                                                                      jint i04, jint i05, jint i06, jint i07,
+                                                                      jint i08, jint i09, jint i0a, jint i0b,
+                                                                      jint i0c, jint i0d, jint i0e, jint i0f,
+                                                                      jint i10, jint i11, jint i12, jint i13,
+                                                                      jint i14, jint i15, jint i16, jint i17,
+                                                                      jint i18, jint i19, jint i1a, jint i1b,
+                                                                      jint i1c, jint i1d, jint i1e, jint i1f,
+                                                                      jint a) {
+  return I32I(i00, i01, i02, i03, i04, i05, i06, i07,
+              i08, i09, i0a, i0b, i0c, i0d, i0e, i0f,
+              i10, i11, i12, i13, i14, i15, i16, i17,
+              i18, i19, i1a, i1b, i1c, i1d, i1e, i1f,
+              a);
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/NativeCallTest.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/NativeCallTest.java
@@ -126,6 +126,26 @@ public void testI32SDILDS() {
         test("I32SDILDS", getI32SDILDS(), float.class, argClazz, argValues);
     }
 
+    @Test
+    public void testI32I() {
+        int sCount = 32;
+        // Pairs of <Object>, <Class>
+        Object[] remainingArgs = new Object[]{
+                        12, int.class
+        };
+        Class<?>[] argClazz = new Class[sCount + remainingArgs.length / 2];
+        Object[] argValues = new Object[sCount + remainingArgs.length / 2];
+        for (int i = 0; i < sCount; i++) {
+            argValues[i] = i;
+            argClazz[i] = int.class;
+        }
+        for (int i = 0; i < remainingArgs.length; i += 2) {
+            argValues[sCount + i / 2] = remainingArgs[i + 0];
+            argClazz[sCount + i / 2] = (Class<?>) remainingArgs[i + 1];
+        }
+        test("I32I", getI32I(), int.class, argClazz, argValues);
+    }
+
     public void test(String name, long addr, Class<?> returnClazz, Class<?>[] types, Object[] values) {
         try {
             test(asm -> {
@@ -245,4 +265,24 @@ public static float L32SDILDS(long l00, long l01, long l02, long l03, long l04,
                         l18, l19, l1a, l1b, l1c, l1d, l1e, l1f,
                         a, b, c, d, e, f);
     }
+
+    public static native long getI32I();
+
+    public static native int _I32I(int i00, int i01, int i02, int i03, int i04, int i05, int i06, int i07,
+                    int i08, int i09, int i0a, int i0b, int i0c, int i0d, int i0e, int i0f,
+                    int i10, int i11, int i12, int i13, int i14, int i15, int i16, int i17,
+                    int i18, int i19, int i1a, int i1b, int i1c, int i1d, int i1e, int i1f,
+                    int a);
+
+    public static int I32I(int i00, int i01, int i02, int i03, int i04, int i05, int i06, int i07,
+                    int i08, int i09, int i0a, int i0b, int i0c, int i0d, int i0e, int i0f,
+                    int i10, int i11, int i12, int i13, int i14, int i15, int i16, int i17,
+                    int i18, int i19, int i1a, int i1b, int i1c, int i1d, int i1e, int i1f,
+                    int a) {
+        return _I32I(i00, i01, i02, i03, i04, i05, i06, i07,
+                    i08, i09, i0a, i0b, i0c, i0d, i0e, i0f,
+                    i10, i11, i12, i13, i14, i15, i16, i17,
+                    i18, i19, i1a, i1b, i1c, i1d, i1e, i1f,
+                    a);
+    }
 }
diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/aarch64/AArch64TestAssembler.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/aarch64/AArch64TestAssembler.java
@@ -272,8 +272,7 @@ public void emitEpilogue() {
 
     @Override
     public void emitCallPrologue(CallingConvention cc, Object... prim) {
-        emitGrowStack(cc.getStackSize());
-        frameSize += cc.getStackSize();
+        growFrame(cc.getStackSize());
         AllocatableValue[] args = cc.getArguments();
         for (int i = 0; i < args.length; i++) {
             emitLoad(args[i], prim[i]);
@@ -282,8 +281,7 @@ public void emitCallPrologue(CallingConvention cc, Object... prim) {
 
     @Override
     public void emitCallEpilogue(CallingConvention cc) {
-        emitGrowStack(-cc.getStackSize());
-        frameSize -= cc.getStackSize();
+        growFrame(-cc.getStackSize());
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ private static RegisterConfig createRegisterConfig(AArch64HotSpotVMConfig config`
`93`	`93`	`// ARMv8 defines r18 as being available to the platform ABI. Windows`
`94`	`94`	`// and Darwin use it for such. Linux doesn't assign it and thus r18 can`
`95`	`95`	`// be used as an additional register.`
`96`		`- boolean canUsePlatformRegister = config.linuxOs;`
	`96`	`+ boolean canUsePlatformRegister = target.linuxOs;`
`97`	`97`	`return new AArch64HotSpotRegisterConfig(target, config.useCompressedOops, canUsePlatformRegister);`
`98`	`98`	`}`
`99`	`99`