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

Tue Feb 24 05:34:49 UTC 2015

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.

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150223/03411d74/attachment-0001.html>