RFR: 8168503 JEP 297: Unified arm32/arm64 Port

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Bob Vandette
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

Webrev:

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

Thanks,
Bob Vandette

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Vladimir Kozlov
hi Bob,

I would suggest to have separate webrevs for different repositories because different groups should look on them.

For example, top repository and makefiles changes should be also reviewed on [hidden email]

Why do you need cahnges in SA libproc.h?

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
>
> Webrev:
>
> 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
>
> Thanks,
> Bob Vandette
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Bob Vandette

> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <[hidden email]> 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.

>
> For example, top repository and makefiles changes should be also reviewed on [hidden email]
>
> 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
>>
>> Webrev:
>>
>> 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
>>
>> Thanks,
>> Bob Vandette
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

coleen.phillimore

The interpreter and runtime parts of this change look good to me, and I
spot checked the arm code.
Coleen


On 12/5/16 10:23 AM, Bob Vandette wrote:

>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <[hidden email]> 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.
>
>> For example, top repository and makefiles changes should be also reviewed on [hidden email]
>>
>> 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
>>>
>>> Webrev:
>>>
>>> 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
>>>
>>> Thanks,
>>> Bob Vandette
>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Edward Nevill-5
In reply to this post by Bob Vandette
On Fri, 2016-12-02 at 15:28 -0500, Bob Vandette wrote:
> Please review the proposed changes to be integrated under JEP 297.


Looks good. I have run it through JTreg on x86, arm32 and arm64 systems. Results below. The repo I used was the 'hs' repo.

On x86 I did two runs, original repo and patched with your webrev. For arm64 I did two runs, one the arm64 unified build (ie the webrev below) and one, the existing aarch64 build.

All builds were release builds.

For the jdk tests I used the options -k:\!headful\&\!intermittant\&\!randomness to suppress any tests requiring X11.

I will append these results to a comment in the issue and start running jcstress.

All the best,
Ed.

x86(original):

hotspot: passed: 1,337; failed: 5; error: 44 (0 fatal errors)*
langtools: passed: 3,757; failed: 5; error: 8 (0 fatal errors)
jdk: passed: 6,348; failed: 194; error: 11 (1 fatal error)

x86(patched):

hotspot: passed: 1,337; failed: 5; error: 44 (0 fatal errors)
langtools: passed: 3,757; failed: 5; error: 8 (0 fatal errors)
jdk: passed: 6,346; failed: 196; error: 10 (0 fatal errors)

arm32

hotspot: passed: 1,244; failed: 5; error: 44 (0 fatal errors)
langtools: passed: 3,761; failed: 1; error: 8 (0 fatal errors)
jdk: passed: 6,339; failed: 197; error: 11 (0 fatal errors)

aarch64

hotspot: passed: 1,325; failed: 9; error: 44 (0 fatal errors)
langtools: passed: 3,757; failed: 2; error: 8 (0 fatal errors)
jdk: passed: 6,389; failed: 155; error: 9 (0 fatal errors)

arm64

hotspot: passed: 1,327; failed: 7; error: 44 (0 fatal errors)
langtools: passed 3,760; failed: 1; error: 8 (0 fatal errors)
jdk: passed: 6,408; failed: 136; error: 9 (0 fatal errors)

* 'fatal' errors are those where the test failed with the message "A fatal error has been detected" in the .jtr file.

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Vladimir Kozlov
In reply to this post by Bob Vandette
On 12/5/16 7:23 AM, Bob Vandette wrote:
>
>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <[hidden email]> 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.

+3 Hotspot makefile cahnges. + new jvm.cfg

But you are right that makefiles changes are first in the webrev and easy to find/review.

>
>>
>> For example, top repository and makefiles changes should be also reviewed on [hidden email]
>>
>> 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.

Please, do this.

Thanks,
Vladimir

>
> 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
>>>
>>> Webrev:
>>>
>>> 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
>>>
>>> Thanks,
>>> Bob Vandette
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Erik Joelsson
In reply to this post by Bob Vandette
Build changes look ok to me.

/Erik


On 2016-12-05 16:23, Bob Vandette wrote:

>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <[hidden email]> 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.
>
>> For example, top repository and makefiles changes should be also reviewed on [hidden email]
>>
>> 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
>>>
>>> Webrev:
>>>
>>> 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
>>>
>>> Thanks,
>>> Bob Vandette
>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

David Holmes
In reply to this post by Bob Vandette
This all looks good to me Bob!

Thanks.

David

On 3/12/2016 6:28 AM, 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
>
> Webrev:
>
> 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
>
> Thanks,
> Bob Vandette
>
Reply | Threaded
Open this post in threaded view
|

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

Magnus Ihse Bursie
In reply to this post by Bob Vandette
On 2016-12-05 16:23, Bob Vandette wrote:
>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <[hidden email]> 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.

* 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.)

* 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.

/Magnus


>
>> For example, top repository and makefiles changes should be also reviewed on [hidden email]
>>
>> 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
>>>
>>> Webrev:
>>>
>>> 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
>>>
>>> Thanks,
>>> Bob Vandette
>>>

Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
Thanks Magnus,  see comments below ..

> On Dec 7, 2016, at 3:44 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>
> On 2016-12-05 16:23, Bob Vandette wrote:
>>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov <[hidden email]> <mailto:[hidden email]> 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 [hidden email] <mailto:[hidden email]>
>>>
>>> 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
>>>>
>

Reply | Threaded
Open this post in threaded view
|

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

Magnus Ihse Bursie
On 2016-12-07 19:51, Bob Vandette wrote:
> Thanks Magnus,  see comments below ..
Your fixes look good. I'm ok with the build part of the patch now.

/Magnus

>
>> On Dec 7, 2016, at 3:44 AM, Magnus Ihse Bursie
>> <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> On 2016-12-05 16:23, Bob Vandette wrote:
>>>> On Dec 2, 2016, at 8:04 PM, Vladimir Kozlov<[hidden email]>  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 [hidden email]
>>>>
>>>> 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
>>>>>
>>>>> Webrev:
>>>>>
>>>>> 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
>>>>>
>>>>> Thanks,
>>>>> Bob Vandette
>>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8168503 JEP 297: Unified arm32/arm64 Port

Edward Nevill-5
In reply to this post by Bob Vandette
On Fri, 2016-12-02 at 15:28 -0500, Bob Vandette wrote:
>
> JEP 297:
>
> https://bugs.openjdk.java.net/browse/JDK-8168503
>
> Webrev:
>
> http://cr.openjdk.java.net/~bobv/8168503
>

I have added the results of JCStress testing as a comment to the JEP.

Basically 100% pass with some 'formally acceptable but surprising' results.

Thanks to Stuart Monteith for helping with the arm32 testing.

All the best,
Ed.

Reply | Threaded
Open this post in threaded view
|

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

Andrew Haley
In reply to this post by Bob Vandette
I've just noticed a nasty problem.  "java -version" on AArch64 gives
no clue about which version of HotSpot, Oracle's or the aarch64-port
project, is running.  I hadn't realized that the version wasn't going
to appear anywhere.

And all that a crash says is

#  SIGSEGV (0xb) at pc=0x0000007fb7a0c2e4, pid=44736, tid=44737
#
# JRE version:  (9.0) (build )
# Java VM: OpenJDK 64-Bit Server VM (9-internal+0-adhoc.aph.hs, mixed mode, tiered, compressed oops, g1 gc, linux-aarch64)

There is a bit more in the log:

---------------  S U M M A R Y ------------
Command Line:
Host: AArch64 Processor rev 0 (aarch64), 48 cores, 62G, Random Linux Distro

I guess the Oracle proprietary release will have the Oracle copyright
etc., so that one wil be clear enough, and if it's from a Linux distro
we'll know which port they use, unless some distro (Gentoo?  Debian?)
is crazy enough to package both versions, in which case it's their
problem.

I had assumed that Oracle's port would call itself "arm64" or
somesuch, to distinguish it.

Even the bug database only has "aarch32" and "aarch64", so it's going
to be crazy hard to distinguish which port has a bug.  We should
really get that fixed.

It's funny how this stuff comes out of the woodwork.

Andrew.
Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
I agree that this is an issue but I’m not sure that it’s a show stopper.

The Oracle build will not have OpenJDK in the version string which will help to differentiate
our binaries from OpenJDK builds.

The bug database field that I think you are describing is only an indication of the architecture
that a bug can be reproduced on.  It is not meant to describe the sources that were used to produce
the binaries or where the binaries came from.  That should to be specified elsewhere in the bug report.

I don’t like the idea of listing arm64 in the version string since we are only using arm64 internally to
trigger the use of the hotspot “arm” directory.  We’d also end up with lots of incorrect bug entries since folks
will fail to use arm64 to report a bug in the Oracle 64-bit ARM port running on an aarch64 based system.

If we start putting build configuration information in the version string, then where do we stop.

Bob.

> On Mar 16, 2017, at 12:50 PM, Andrew Haley <[hidden email]> wrote:
>
> I've just noticed a nasty problem.  "java -version" on AArch64 gives
> no clue about which version of HotSpot, Oracle's or the aarch64-port
> project, is running.  I hadn't realized that the version wasn't going
> to appear anywhere.
>
> And all that a crash says is
>
> #  SIGSEGV (0xb) at pc=0x0000007fb7a0c2e4, pid=44736, tid=44737
> #
> # JRE version:  (9.0) (build )
> # Java VM: OpenJDK 64-Bit Server VM (9-internal+0-adhoc.aph.hs, mixed mode, tiered, compressed oops, g1 gc, linux-aarch64)
>
> There is a bit more in the log:
>
> ---------------  S U M M A R Y ------------
> Command Line:
> Host: AArch64 Processor rev 0 (aarch64), 48 cores, 62G, Random Linux Distro
>
> I guess the Oracle proprietary release will have the Oracle copyright
> etc., so that one wil be clear enough, and if it's from a Linux distro
> we'll know which port they use, unless some distro (Gentoo?  Debian?)
> is crazy enough to package both versions, in which case it's their
> problem.
>
> I had assumed that Oracle's port would call itself "arm64" or
> somesuch, to distinguish it.
>
> Even the bug database only has "aarch32" and "aarch64", so it's going
> to be crazy hard to distinguish which port has a bug.  We should
> really get that fixed.
>
> It's funny how this stuff comes out of the woodwork.
>
> Andrew.

Reply | Threaded
Open this post in threaded view
|

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

Andrew Haley
On 16/03/17 18:03, Bob Vandette wrote:

> I agree that this is an issue but I’m not sure that it’s a show
> stopper.
>
> The Oracle build will not have OpenJDK in the version string which
> will help to differentiate our binaries from OpenJDK builds.

Right, like I said.

> The bug database field that I think you are describing is only an
> indication of the architecture that a bug can be reproduced on.  It
> is not meant to describe the sources that were used to produce the
> binaries or where the binaries came from.  That should to be
> specified elsewhere in the bug report.

OK.  I would surely have tried to insist that the version strings were
different for our two ports at the time your port was committed, but I
blew my chance.

> I don’t like the idea of listing arm64 in the version string since
> we are only using arm64 internally to trigger the use of the hotspot
> “arm” directory.  We’d also end up with lots of incorrect bug
> entries since folks will fail to use arm64 to report a bug in the
> Oracle 64-bit ARM port running on an aarch64 based system.
>
> If we start putting build configuration information in the version
> string, then where do we stop.

It's going to be rather horrible, though.  How do we reproduce a bug
if we don't know what port is causing the bug?  How do we even ask the
question if we don't know what the ports are called?  I always assumed
we were "aarch64" and you were "arm64".  How are we to ask a user if
we can't tell them what to look for?

Even if we don't change anything in OpenJDK itself, we'll still have
to agree on a label to use in the bug database.  I don't know what
labels we should use, but we should agree on them now.  Do you have
any preferences?

Andrew.
Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette

> On Mar 16, 2017, at 2:27 PM, Andrew Haley <[hidden email]> wrote:
>
> On 16/03/17 18:03, Bob Vandette wrote:
>
>> I agree that this is an issue but I’m not sure that it’s a show
>> stopper.
>>
>> The Oracle build will not have OpenJDK in the version string which
>> will help to differentiate our binaries from OpenJDK builds.
>
> Right, like I said.
>
>> The bug database field that I think you are describing is only an
>> indication of the architecture that a bug can be reproduced on.  It
>> is not meant to describe the sources that were used to produce the
>> binaries or where the binaries came from.  That should to be
>> specified elsewhere in the bug report.
>
> OK.  I would surely have tried to insist that the version strings were
> different for our two ports at the time your port was committed, but I
> blew my chance.
>
>> I don’t like the idea of listing arm64 in the version string since
>> we are only using arm64 internally to trigger the use of the hotspot
>> “arm” directory.  We’d also end up with lots of incorrect bug
>> entries since folks will fail to use arm64 to report a bug in the
>> Oracle 64-bit ARM port running on an aarch64 based system.
>>
>> If we start putting build configuration information in the version
>> string, then where do we stop.
>
> It's going to be rather horrible, though.  How do we reproduce a bug
> if we don't know what port is causing the bug?  How do we even ask the
> question if we don't know what the ports are called?  I always assumed
> we were "aarch64" and you were "arm64".  How are we to ask a user if
> we can't tell them what to look for?
>
> Even if we don't change anything in OpenJDK itself, we'll still have
> to agree on a label to use in the bug database.  I don't know what
> labels we should use, but we should agree on them now.  Do you have
> any preferences?

I agree that a label would be very useful.  For this purpose, I’m not opposed
to using the arm64 versus aarch64 names.   Let me check around to see
if anyone has a better suggestion.

Bob.



>
> Andrew.

Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
I checked with the hotspot compiler team and their manager and they are ok with
using the labels “arm64” and “aarch64” to mark bugs that are specific to 64-bit aarch64
builds that are done using hotspot/src/cpu/arm versus hotspot/src/cpu/aarch64 sources.

Please publicize this label’s use throughout interested OpenJDK developers.

Bob.


> On Mar 16, 2017, at 2:40 PM, Bob Vandette <[hidden email]> wrote:
>
>
>> On Mar 16, 2017, at 2:27 PM, Andrew Haley <[hidden email]> wrote:
>>
>> On 16/03/17 18:03, Bob Vandette wrote:
>>
>>> I agree that this is an issue but I’m not sure that it’s a show
>>> stopper.
>>>
>>> The Oracle build will not have OpenJDK in the version string which
>>> will help to differentiate our binaries from OpenJDK builds.
>>
>> Right, like I said.
>>
>>> The bug database field that I think you are describing is only an
>>> indication of the architecture that a bug can be reproduced on.  It
>>> is not meant to describe the sources that were used to produce the
>>> binaries or where the binaries came from.  That should to be
>>> specified elsewhere in the bug report.
>>
>> OK.  I would surely have tried to insist that the version strings were
>> different for our two ports at the time your port was committed, but I
>> blew my chance.
>>
>>> I don’t like the idea of listing arm64 in the version string since
>>> we are only using arm64 internally to trigger the use of the hotspot
>>> “arm” directory.  We’d also end up with lots of incorrect bug
>>> entries since folks will fail to use arm64 to report a bug in the
>>> Oracle 64-bit ARM port running on an aarch64 based system.
>>>
>>> If we start putting build configuration information in the version
>>> string, then where do we stop.
>>
>> It's going to be rather horrible, though.  How do we reproduce a bug
>> if we don't know what port is causing the bug?  How do we even ask the
>> question if we don't know what the ports are called?  I always assumed
>> we were "aarch64" and you were "arm64".  How are we to ask a user if
>> we can't tell them what to look for?
>>
>> Even if we don't change anything in OpenJDK itself, we'll still have
>> to agree on a label to use in the bug database.  I don't know what
>> labels we should use, but we should agree on them now.  Do you have
>> any preferences?
>
> I agree that a label would be very useful.  For this purpose, I’m not opposed
> to using the arm64 versus aarch64 names.   Let me check around to see
> if anyone has a better suggestion.
>
> Bob.
>
>
>
>>
>> Andrew.
>

Reply | Threaded
Open this post in threaded view
|

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

Andrew Haley
In reply to this post by Bob Vandette
On 16/03/17 18:03, Bob Vandette wrote:
> If we start putting build configuration information in the version string, then where do we stop.

Two separate ports is pretty large for a "build configuration issue".

How about we define a unique global dynamic symbol?  Then, at least,
it's just a matter of using nm or equivalent to have a look at which
port we have.

Andrew.

Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
I’d prefer to use something like these System properties.

"java.vendor" JRE vendor name
"java.vendor.url" JRE vendor URL

 or to add something to the “release” file.

HOTSPOT_ARCH_DIR=arm

or

HOTSPOT_ARCH_DIR=aarch64

Bob.


> On Mar 21, 2017, at 10:26 AM, Andrew Haley <[hidden email]> wrote:
>
> On 16/03/17 18:03, Bob Vandette wrote:
>> If we start putting build configuration information in the version string, then where do we stop.
>
> Two separate ports is pretty large for a "build configuration issue".
>
> How about we define a unique global dynamic symbol?  Then, at least,
> it's just a matter of using nm or equivalent to have a look at which
> port we have.
>
> Andrew.
>

Reply | Threaded
Open this post in threaded view
|

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

Andrew Haley
On 21/03/17 15:25, Bob Vandette wrote:

> I’d prefer to use something like these System properties.
>
> "java.vendor" JRE vendor name
> "java.vendor.url" JRE vendor URL
>
>  or to add something to the “release” file.
>
> HOTSPOT_ARCH_DIR=arm
>
> or
>
> HOTSPOT_ARCH_DIR=aarch64

OK, sounds good.  I'll have a think.

Andrew.

12