Skip to content

8254827: JVMCI: Enable it for Windows+AArch64 #307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

8254827: JVMCI: Enable it for Windows+AArch64 #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions make/autoconf/hotspot.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ AC_DEFUN_ONCE([HOTSPOT_SETUP_JVM_FEATURES],
# Only enable jvmci on x86_64, sparcv9 and aarch64
if test "x$OPENJDK_TARGET_CPU" = "xx86_64" || \
test "x$OPENJDK_TARGET_CPU" = "xsparcv9" || \
test "x$OPENJDK_TARGET_OS-$OPENJDK_TARGET_CPU" = "xlinux-aarch64" ; then
test "x$OPENJDK_TARGET_CPU" = "xaarch64" ; then
AC_MSG_RESULT([yes])
JVM_FEATURES_jvmci="jvmci"
INCLUDE_JVMCI="true"
Expand Down Expand Up @@ -444,10 +444,11 @@ AC_DEFUN_ONCE([HOTSPOT_SETUP_JVM_FEATURES],
JVM_FEATURES_graal="graal"
INCLUDE_GRAAL="true"
else
# By default enable graal build on x64 or where AOT is available.
# By default enable graal build on x64/aarch64 or where AOT is available.
# graal build requires jvmci.
if test "x$JVM_FEATURES_jvmci" = "xjvmci" && \
(test "x$OPENJDK_TARGET_CPU" = "xx86_64" || \
test "x$OPENJDK_TARGET_CPU" = "xaarch64" || \
test "x$ENABLE_AOT" = "xtrue") ; then
AC_MSG_RESULT([yes])
JVM_FEATURES_graal="graal"
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/cpu/aarch64/jvmciCodeInstaller_aarch64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* questions.
*/

#include "precompiled.hpp"
#include "asm/macroAssembler.hpp"
#include "jvmci/jvmciCodeInstaller.hpp"
#include "jvmci/jvmciRuntime.hpp"
Expand Down
6 changes: 5 additions & 1 deletion ...m.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotJVMCIBackendFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ protected HotSpotConstantReflectionProvider createConstantReflection(HotSpotJVMC
}

protected RegisterConfig createRegisterConfig(AArch64HotSpotVMConfig config, TargetDescription target) {
return new AArch64HotSpotRegisterConfig(target, config.useCompressedOops);
// 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;
return new AArch64HotSpotRegisterConfig(target, config.useCompressedOops, canUsePlatformRegister);
}

protected HotSpotCodeCacheProvider createCodeCache(HotSpotJVMCIRuntime runtime, TargetDescription target, RegisterConfig regConfig) {
Expand Down
18 changes: 14 additions & 4 deletions ...jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import static jdk.vm.ci.aarch64.AArch64.rscratch1;
import static jdk.vm.ci.aarch64.AArch64.rscratch2;
import static jdk.vm.ci.aarch64.AArch64.r12;
import static jdk.vm.ci.aarch64.AArch64.r18;
import static jdk.vm.ci.aarch64.AArch64.r27;
import static jdk.vm.ci.aarch64.AArch64.r28;
import static jdk.vm.ci.aarch64.AArch64.r29;
Expand Down Expand Up @@ -122,16 +123,22 @@ public RegisterAttributes[] getAttributesMap() {
*/
public static final Register metaspaceMethodRegister = r12;

/**
* The platform ABI can use r18 to carry inter-procedural state (e.g. thread
* context). If not defined as such by the platform ABI, it can be used as
* additional temporary register.
*/
public static final Register platformRegister = r18;
public static final Register heapBaseRegister = r27;
public static final Register threadRegister = r28;
public static final Register fp = r29;

private static final RegisterArray reservedRegisters
= new RegisterArray(rscratch1, rscratch2, threadRegister, fp, lr, r31, zr, sp);

private static RegisterArray initAllocatable(Architecture arch, boolean reserveForHeapBase) {
private static RegisterArray initAllocatable(Architecture arch, boolean reserveForHeapBase, boolean canUsePlatformRegister) {
RegisterArray allRegisters = arch.getAvailableValueRegisters();
Register[] registers = new Register[allRegisters.size() - reservedRegisters.size() - (reserveForHeapBase ? 1 : 0)];
Register[] registers = new Register[allRegisters.size() - reservedRegisters.size() - (reserveForHeapBase ? 1 : 0) - (!canUsePlatformRegister ? 1 : 0)];
Copy link
Contributor

@theRealAph theRealAph Oct 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing and complicated. If you set platformRegister to be either r18 or Register.None depending on the OS, you don't need a separate boolean canUsePlatformRegister.

Copy link
Member

@lewurm lewurm Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it doesn't read nice. However the code around the definition of platformRegister doesn't know which platform we are dealing with. We could potentially pass down the config object via the constructor... or use the boolean flag as proposed here. Do you think it's worth changing it? I'm mostly worried about diverging from tip.

Copy link
Contributor

@theRealAph theRealAph Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see. Let it stand.

List<Register> reservedRegistersList = reservedRegisters.asList();

int idx = 0;
Expand All @@ -140,6 +147,9 @@ private static RegisterArray initAllocatable(Architecture arch, boolean reserveF
// skip reserved registers
continue;
}
if (!canUsePlatformRegister && reg.equals(platformRegister)) {
continue;
}
assert !(reg.equals(threadRegister) || reg.equals(fp) || reg.equals(lr) || reg.equals(r31) || reg.equals(zr) || reg.equals(sp));
if (reserveForHeapBase && reg.equals(heapBaseRegister)) {
// skip heap base register
Expand All @@ -153,8 +163,8 @@ private static RegisterArray initAllocatable(Architecture arch, boolean reserveF
return new RegisterArray(registers);
}

public AArch64HotSpotRegisterConfig(TargetDescription target, boolean useCompressedOops) {
this(target, initAllocatable(target.arch, useCompressedOops));
public AArch64HotSpotRegisterConfig(TargetDescription target, boolean useCompressedOops, boolean canUsePlatformRegister) {
this(target, initAllocatable(target.arch, useCompressedOops, canUsePlatformRegister));
assert callerSaved.size() >= allocatable.size();
}

Expand Down