RFR(M) 8072383: resolve conflicts between open and closed ports

David Holmes david.holmes at oracle.com
Mon Feb 23 00:18:24 UTC 2015


Hi Volker,

On 21/02/2015 3:57 AM, Volker Simonis wrote:
> I've tested the jdk9 patch now and it's good that I did it :)
>
> It works on AIX [3] but there's a subtle problem in your patch which
> lets the build fail on Linux/ppc64 (and caused my quite some headache
> :)
>
> The failure is because of a peculiarity introduced by 8046471 [1,2]
> (done by Mikael which is on CC now). After that change ARCH will be
> set to "ppc" for complete top-level builds but it will be "ppc64" for
> hotspot-only builds started from the hotspot/make subdirectory. I
> don't remember the exact reasoning why 8046471 changed the value of
> OPENJDK_TARGET_CPU_ARCH from "ppc64" to "ppc" spec.gmk (maybe Mikael
> can help) which finally led to the definition of ARCH=ppc in
> hotspot-spec.gmk.

The 8046471 change replaced:

ARCH=$(OPENJDK_TARGET_CPU_LEGACY)

with

ARCH=$(OPENJDK_TARGET_CPU_ARCH)

but for non x86 the default for OPENJDK_TARGET_CPU_LEGACY is 
OPENJDK_TARGET_CPU not OPENJDK_TARGET_CPU_ARCH. Hence this caused the 
change from ppc64 to ppc. That wasn't flagged as an issue in your review 
of that bug - though perhaps it simply wasn't noticed given hotspot 
ignored it anyway for AIX.

Between this and the proposed ppc64le changes I'm finding this whole 
"arch" setup extremely confusing. The use of CPU versus CPU_ARCH is very 
unclear. :(

David
-----

> But because of that change, which was only applied to jdk9 and not
> down-ported to jdk8u, the build now fails on Linux/ppc64 with your
> change applied. It can be easily fixed by the following small patch
> (which actually just reverts two of your changes):
>
>   diff -r f1881e4d4ad6 make/defs.make
> --- a/make/defs.make    Fri Feb 20 14:53:58 2015 +0100
> +++ b/make/defs.make    Fri Feb 20 18:17:27 2015 +0100
> @@ -286,7 +286,7 @@
>
>     # Use uname output for SRCARCH, but deal with platform differences. If ARCH
>     # is not explicitly listed below, it is treated as x86.
> -  SRCARCH    ?= $(ARCH/$(filter sparc sparc64 ia64 amd64 x86_64 ppc64
> aarch64 zero,$(ARCH)))
> +  SRCARCH    ?= $(ARCH/$(filter sparc sparc64 ia64 amd64 x86_64 ppc
> ppc64 aarch64 zero,$(ARCH)))
>     ARCH/       = x86
>     ARCH/sparc  = sparc
>     ARCH/sparc64= sparc
> @@ -294,6 +294,7 @@
>     ARCH/amd64  = x86
>     ARCH/x86_64 = x86
>     ARCH/ppc64  = ppc
> +  ARCH/ppc    = ppc
>     ARCH/aarch64= aarch64
>     ARCH/zero   = zero
>
> I'd propose to incorporate this small patch into your change. If
> that's not acceptable for you, we would have to change the top-level
> arch-detection and possibly revert 8046471 for ppc which seems much
> more complicated to me.
>
> Regards,
> Volker
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8046471
> [2] http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/rev/0a039fc78645
> [3] because we unconditionally set ARCH to "ppc64" in
> make/aix/makefiles/defs.make and ignore the ARCH value from the
> top-level hotspot-spec.gmk file.
>
> On Fri, Feb 20, 2015 at 2:32 PM, Volker Simonis
> <volker.simonis at gmail.com> wrote:
>> Hi Dean,
>>
>> I've only looked at the 8u60 changes until now but it's actually a
>> good idea to also check the aarch-stage repo on our platforms.
>> I'll run the builds and let you know.
>>
>> Regards,
>> Volker
>>
>>
>> On Thu, Feb 19, 2015 at 10:40 PM, Dean Long <dean.long at oracle.com> wrote:
>>> Thanks Volker, I will list you as a reviewer for 8u60.  Did you also look at
>>> the 9
>>> changes?  If so I will list you as a reviewer for 9 as well.
>>>
>>> dl
>>>
>>>
>>> On 2/19/2015 2:27 AM, Volker Simonis wrote:
>>>>
>>>> Hi Dean,
>>>>
>>>> I've just checked webrev.8u60.01 on Linux/PPC64. It cleanly builds and
>>>> runs so thumbs up from me.
>>>>
>>>> Regards,
>>>> Volker
>>>>
>>>>
>>>> On Thu, Feb 19, 2015 at 5:12 AM, Dean Long <dean.long at oracle.com> wrote:
>>>>>
>>>>> Yes, good catch.  I've update the webrev to webrev.8u60.01.  Thanks for
>>>>> the
>>>>> review.
>>>>>
>>>>> dl
>>>>>
>>>>>
>>>>> On 2/18/2015 4:45 PM, Vladimir Kozlov wrote:
>>>>>>
>>>>>> Did you missed to remove #elif ppc_32 in 8u60 changes in
>>>>>> src/share/vm/interpreter/templateTable.hpp?
>>>>>>
>>>>>> Otherwise both versions look good.
>>>>>>
>>>>>> Thanks,
>>>>>> Vlaidmir
>>>>>>
>>>>>> On 2/18/15 4:26 PM, Dean Long wrote:
>>>>>>>
>>>>>>> Just to avoid confusion, let me clarify that the jdk9 changes below
>>>>>>> can't be pushed until after the open aarch64 port is merged into the
>>>>>>> main jdk9, as they are based on the staging repo.
>>>>>>>
>>>>>>> dl
>>>>>>>
>>>>>>> On 2/18/2015 4:02 PM, Dean Long wrote:
>>>>>>>>
>>>>>>>> These changes resolve some issues with references to closed ports in
>>>>>>>> open hotspot code,
>>>>>>>> primarily by removing those references completely.  I have included
>>>>>>>> the 8u60 backport
>>>>>>>> as well because it won't apply cleanly, and I may push it first
>>>>>>>> because the 9 changes are
>>>>>>>> blocked by JEP 237: Linux/AArch64 Port.
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~dlong/8072383/webrev.8u60.00/
>>>>>>>> http://cr.openjdk.java.net/~dlong/8072383/webrev.9.00/
>>>>>>>>
>>>>>>>> dl
>>>>>>>
>>>>>>>
>>>


More information about the hotspot-compiler-dev mailing list