RFR(M) 8072383: resolve conflicts between open and closed ports
Dean Long
dean.long at oracle.com
Tue Feb 24 20:00:14 UTC 2015
On 2/24/2015 5:31 AM, Volker Simonis wrote:
> On Tue, Feb 24, 2015 at 6:34 AM, Dean Long <dean.long at oracle.com> wrote:
>> Hi David. Thanks for the reviews. I removed the linux-ppc logic in
>> make/linux/makefiles/defs.make for both jdk9 and 8u60. I also
>> updated the copyrights.
>>
>> Volker, I don't think this last linux-ppc change will affect
>> linux-ppc64, but perhaps you can double-check it.
> Hi Dean,
>
> I've just checked your latest 8 and 9 patches on Linux/ppc64 and AIX
> and couldn't find any issues.
> So from my side everything is fine.
Great! Thanks again Volker.
dl
> Regards,
> Volker
>
>> thanks,
>>
>> dl
>>
>> http://cr.openjdk.java.net/~dlong/8072383/webrev.8u60.02/
>> http://cr.openjdk.java.net/~dlong/8072383/webrev.9.02/
>>
>>
>> On 2/22/2015 4:52 PM, David Holmes wrote:
>>
>> Hi Dean,
>>
>> Sorry for the delay on this.
>>
>> On 21/02/2015 5:02 AM, Dean Long wrote:
>>
>> Thanks Volker for catching this. That was one of the issues I was
>> worried about. I have incorporated your patch. Webrev updated to
>> webrev.9.01.
>>
>>
>> So even with the ppc vs ppc64 confusion, in
>> webrev.9.01/make/linux/makefiles/defs.make you should be able to remove the
>> 32-bit ppc logic:
>>
>> 124 else
>> 125 ARCH_DATA_MODEL = 32
>> 126 PLATFORM = linux-ppc
>> 127 VM_PLATFORM = linux_ppc
>>
>> Otherwise jdk9 version looks okay to me, just needs copyright dates updated.
>> Also I just want to mention:
>>
>> webrev.9.01/src/os/linux/vm/os_linux.cpp
>>
>> // Cpu architecture string
>> ! static char cpu_arch[] = HOTSPOT_LIB_ARCH;
>>
>> 10 bonus points for this cleanup! :) Now if only we could do something
>> similar for #include's ... ;-) We should also be able to do something
>> similar for src/share/vm/runtime/vm_version.cpp CPU (and OS) value!
>>
>> ---
>>
>> Looking at 8u60:
>>
>> make/linux/makefiles/defs.make
>>
>> Can't the 32-bit PPC logic go now?
>>
>> 120 # PPC
>> 121 ifeq ($(ARCH), ppc)
>> 122 ARCH_DATA_MODEL = 32
>> 123 PLATFORM = linux-ppc
>> 124 VM_PLATFORM = linux_ppc
>> 125 HS_ARCH = ppc
>> 126 endif
>>
>> ---
>>
>> src/share/vm/c1/c1_LIR.hpp
>>
>> 620 #if defined(C1_LIR_MD_HPP)
>> 621 # include C1_LIR_MD_HPP
>> 622 #elif defined(SPARC)
>>
>> includes in the middle of code like this is really ugly. While I favour
>> moving platform specific code to platform specific files I think things
>> needs to be refactored so that we don't need this kind of hack. For now,
>> given the timing constraints, this might be acceptable, but it needs to be
>> cleaned up down the track (all the other platform specific code would
>> ideally be moved to platform specific files). Also unclear why 8u60 and 9
>> differ here.
>>
>> Copyright dates need fixing as well.
>>
>> Thanks,
>> David
>>
>>
>> dl
>>
>> On 2/20/2015 9: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.
>>
>> 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-runtime-dev
mailing list