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

David Holmes david.holmes at oracle.com
Mon Feb 23 00:52:56 UTC 2015


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