[aarch64-port-dev ] RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Bob Vandette bob.vandette at oracle.com
Wed Dec 7 18:51:45 UTC 2016 

Thanks Magnus,  see comments below ..

> On Dec 7, 2016, at 3:44 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
> 
> On 2016-12-05 16:23, Bob Vandette wrote:
>>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> <mailto:vladimir.kozlov at oracle.com> wrote:
>>> 
>>> hi Bob,
>>> 
>>> I would suggest to have separate webrevs for different repositories because different groups should look on them.
>> There are only 3 non hotspot files and they are on top.  Forwarding to build-dev for their review.
> 
> Build changes mostly look good. A few comments:
> 
> * We try to use the autoconf defined "$with_opt" variables only when checking and verifying arguments. In hotspot.m4, please let SETUP_HOTSPOT_TARGET_CPU_PORT assign the user specified value to e.g. OPENJDK_TARGET_CPU_PORT, and use that to do the check in HOTSPOT_SETUP_JVM_FEATURES.

I think it should be called HOTSPOT_TARGET_CPU_PORT since this variable only impact which directory hotspot uses for it build.
I also removed a redundant if test "x$with_cpu_port" != x; in SETUP_HOTSPOT_TARGET_CPU_PORT.

+  # Override hotspot cpu definitions for ARM platforms
+  if test "x$OPENJDK_TARGET_CPU" = xarm; then
+    HOTSPOT_TARGET_CPU=arm_32
+    HOTSPOT_TARGET_CPU_DEFINE="ARM32"
+    JVM_LDFLAGS="$JVM_LDFLAGS -fsigned-char"
+    JVM_CFLAGS="$JVM_CFLAGS -DARM -fsigned-char"
+  elif test "x$OPENJDK_TARGET_CPU" = xaarch64 && test "x$HOTSPOT_TARGET_CPU_PORT" = xarm64; then
+    HOTSPOT_TARGET_CPU=arm_64
+    HOTSPOT_TARGET_CPU_ARCH=arm
+    JVM_LDFLAGS="$JVM_LDFLAGS -fsigned-char"
+    JVM_CFLAGS="$JVM_CFLAGS -DARM -fsigned-char"
+  fi
+

+AC_DEFUN([SETUP_HOTSPOT_TARGET_CPU_PORT],
+[
+  AC_ARG_WITH(cpu-port, [AS_HELP_STRING([--with-cpu-port],
+      [specify sources to use for Hotspot 64-bit ARM port (arm64,aarch64) @<:@aarch64@:>@ ])])
+
+  if test "x$with_cpu_port" != x; then
+    if test "x$OPENJDK_TARGET_CPU" != xaarch64; then
+      AC_MSG_ERROR([--with-cpu-port only available on aarch64])
+    fi
+    if test "x$with_cpu_port" != xarm64 && \
+        test "x$with_cpu_port" != xaarch64; then
+      AC_MSG_ERROR([--with-cpu-port must specify arm64 or aarch64])
+    fi
+    HOTSPOT_TARGET_CPU_PORT = $with_cpu_port
+  fi
+])
+
+

> 
> * In flags.m4, the SET_SHARED_LIBRARY_ORIGIN code checks OPENJDK_TARGET_CPU_ARCH. I'd prefer if it checked OPENJDK_TARGET_CPU instead, since explicitly checking the CPU_ARCH variable seems to indicate that we want to test more than a single CPU, but for arm there is only one. (At first, I thought this was a test for "arm64" as well. If this was the intention, then the code is broken.)
> 
Ok.

> * In CompileJvm.gmk, you might want to rephrase the comment, since from now on both the original aarch64 and the new arm64 port are "open". What the code does is in fact to select between the two different aarch64 ports.

Done.  I also found another comment that needed to be updated.

diff --git a/make/lib/CompileJvm.gmk b/make/lib/CompileJvm.gmk
--- a/make/lib/CompileJvm.gmk
+++ b/make/lib/CompileJvm.gmk
@@ -63,8 +63,8 @@
 # INCLUDE_SUFFIX_* is only meant for including the proper
 # platform files. Don't use it to guard code. Use the value of
 # HOTSPOT_TARGET_CPU_DEFINE etc. instead.
-# Remaining TARGET_ARCH_* is needed to distinguish closed and open
-# 64-bit ARM ports (also called AARCH64).
+# Remaining TARGET_ARCH_* is needed to select the cpu specific 
+# sources for 64-bit ARM ports (arm versus aarch64).
 JVM_CFLAGS_TARGET_DEFINES += \
     -DTARGET_ARCH_$(HOTSPOT_TARGET_CPU_ARCH) \
     -DINCLUDE_SUFFIX_OS=_$(HOTSPOT_TARGET_OS) \
@@ -139,6 +139,20 @@
 ################################################################################
 # Platform specific setup
 
+# ARM source selection
+
+ifeq ($(OPENJDK_TARGET_OS)-$(OPENJDK_TARGET_CPU), linux-arm)
+  JVM_EXCLUDE_PATTERNS += arm_64
+
+else ifeq ($(OPENJDK_TARGET_OS)-$(OPENJDK_TARGET_CPU), linux-aarch64)
+  # For 64-bit arm builds, we use the 64 bit hotspot/src/cpu/arm 
+  # hotspot sources if HOTSPOT_TARGET_CPU_ARCH is set to arm.
+  # Exclude the aarch64 and 32 bit arm files for this build.
+  ifeq ($(HOTSPOT_TARGET_CPU_ARCH), arm)
+    JVM_EXCLUDE_PATTERNS += arm_32 aarch64
+  endif
+endif
+

Bob.


> 
> /Magnus
> 
> 
>> 
>>> For example, top repository and makefiles changes should be also reviewed on build-dev at openjdk.java.net <mailto:build-dev at openjdk.java.net>
>>> 
>>> Why do you need cahnges in SA libproc.h?
>> The cross compilation toolchains we use do not define user_regs_struct or user_pt_regs.
>> 
>> I just looked again and there is a definition of struct user_regs in user.h.  I might be able to change the code to:
>> 
>> #if defined(arm) || defined(arm64)
>> #define user_regs_struct user_regs
>> #endif
>> 
>> This change would result  in the exact same declaration based on the number of registers
>> derived from the structure in user.h. 
>> 
>> Bob.
>> 
>> 
>>> I saw Hotspot changes before and I think they are fine (did not dive deep).
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 12/2/16 12:28 PM, Bob Vandette wrote:
>>>> Please review the proposed changes to be integrated under JEP 297.
>>>> 
>>>> Summary:
>>>> 
>>>> This JEP adds arm32 and arm64 Linux platform support to OpenJDK for JDK 9.
>>>> 
>>>> This changeset also removes the support for the pregenerated interpreter since
>>>> this is no longer supported.
>>>> 
>>>> The addition of arm64 does not replace the existing aarch64 port.  A new configure
>>>> option (-with-cpu-port=) allows for the selection of the existing aarch64 versus the
>>>> 64-bit arm64 support being added via this JEP.  Please refer to the JEP for more details.
>>>> 
>>>> JEP 297:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8168503 <https://bugs.openjdk.java.net/browse/JDK-8168503>
>>>> 
>>>> Webrev:
>>>> 
>>>> http://cr.openjdk.java.net/~bobv/8168503 <http://cr.openjdk.java.net/~bobv/8168503>
>>>> 
>>>> 
>>>> Note:
>>>> 
>>>> A complete build-able forest containing these changes is located here: http://hg.openjdk.java.net/aarch32-port/jdk9-arm3264 <http://hg.openjdk.java.net/aarch32-port/jdk9-arm3264>
>>>> 
>>>> Thanks,
>>>> Bob Vandette
>>>> 
>

More information about the hotspot-dev mailing list