RFR(M): 8166560: [s390] Basic enablement of s390 port.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Mon Oct 10 06:37:59 UTC 2016


Hi David, 

ok, I removed the comment ... 
http://cr.openjdk.java.net/~goetz/wr16/8166560-basic_s390/hotspot.wr04/

Best regards,
  Goetz.

> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Mittwoch, 5. Oktober 2016 04:17
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Volker Simonis
> <volker.simonis at gmail.com>
> Cc: hotspot-dev at openjdk.java.net; core-libs-dev <core-libs-
> dev at openjdk.java.net>
> Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
> 
> On 5/10/2016 2:52 AM, Lindenmaier, Goetz wrote:
> > Hi David,
> >
> > I fixed the change accordingly:
> > http://cr.openjdk.java.net/~goetz/wr16/8166560-
> basic_s390/hotspot.wr03/
> >
> > But do you really want me to incorporate a bugfix for ARM in the port?
> 
> Sure - it is only an error message and we've probably never encountered
> it. All the other code has the case for Aarch64 first so this is no
> different - and no need to comment it as it isn't commented upon
> elsewhere.
> 
> Thanks,
> David
> 
> > Best regards,
> >   Goetz.
> >
> >> -----Original Message-----
> >> From: David Holmes [mailto:david.holmes at oracle.com]
> >> Sent: Dienstag, 4. Oktober 2016 09:56
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Volker Simonis
> >> <volker.simonis at gmail.com>
> >> Cc: hotspot-dev at openjdk.java.net; core-libs-dev <core-libs-
> >> dev at openjdk.java.net>
> >> Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
> >>
> >> Hi Goetz,
> >>
> >> On 28/09/2016 8:26 PM, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> here a new webrev for this change:
> >>> http://cr.openjdk.java.net/~goetz/wr16/8166560-
> >> basic_s390/hotspot.wr02/
> >>>
> >>> I reverted the major reorderings in macros.hpp and os_linux.cpp.
> >>> David asked me to do so, and I guess it makes reviewing more simple.
> >>
> >> Thanks. That said ...
> >>
> >>> Also this fixes the issue spotted by David which actually was wrong.
> >>> The other renaming of ARM to ARM32 was correct, though, as
> >>> AARCH64 is defined in both ARM 64-bit ports, and is checked before.
> >>> So the existing case checking ARM is only reached if !LP64.
> >>> I documented this ...
> >>
> >> ... We actually have a bug with the Elf32_* defines in os_linux.cpp
> >>
> >> 1769 #elif  (defined ARM) // ARM must come before AARCH64 because
> the
> >> closed 64-bit port requires so.
> >> 1770   static  Elf32_Half running_arch_code=EM_ARM;
> >>
> >> Aarch64 should come first otherwise we will set the wrong value. I've
> >> been told it would only affect an error message but still ... can I ask
> >> you to fix this please. Thanks.
> >>
> >> ---
> >> src/share/vm/code/codeCache.cpp
> >>
> >> I'd prefer to just see something like:
> >>
> >> S390_ONLY(if (_heaps == NULL) return;) // May be true when NMT detail
> is
> >> used
> >>
> >> ---
> >>
> >> Otherwise seems okay.
> >>
> >> Thanks,
> >> David
> >>
> >>> Also I removed the change in mutex.hpp.  Maybe we contribute
> >>> the full change this was part of, but independent of the s390 port.
> >>>
> >>> I withdraw the part of the change to jdk introducing jvm.cfg.  Volker
> wants
> >> to
> >>> do a separate change for this.
> >>>
> >>> Best regards,
> >>>   Goetz.
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Volker Simonis [mailto:volker.simonis at gmail.com]
> >>>> Sent: Dienstag, 27. September 2016 19:58
> >>>> To: David Holmes <david.holmes at oracle.com>
> >>>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> >>>> dev at openjdk.java.net; core-libs-dev <core-libs-
> dev at openjdk.java.net>
> >>>> Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
> >>>>
> >>>> On Fri, Sep 23, 2016 at 8:11 AM, David Holmes
> >> <david.holmes at oracle.com>
> >>>> wrote:
> >>>>> Hi Goetz,
> >>>>>
> >>>>> I see a change not related directly to S390 ie change from ARM to
> ARM32
> >> in
> >>>>> src/os/linux/vm/os_linux.cpp
> >>>>>
> >>>>
> >>>> The change looks a little confusing because Goetz reordered the ifdef
> >>>> cascades alphabetically (which I think is good).
> >>>>
> >>>> Besides that, the only real change not related to s390 is indeed the
> >>>> change from ARM to ARM32 which happend two times in the file.
> >>>>
> >>>> @Goetz: have you done this intentionally?
> >>>>
> >>>>> It will be a while before I can go through this in any detail.
> >>>>>
> >>>>> David
> >>>>>
> >>>>>
> >>>>> On 23/09/2016 3:52 PM, Lindenmaier, Goetz wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> This change is part of the s390 port. It contains some basic adaptions
> >>>>>> needed for a full hotspot port for linux s390x.
> >>>>>>
> >>>>>> It defines the required macros, platform names and includes.
> >>>>>>
> >>>>>> The s390 port calles CodeCache::contains() in current_frame(), which
> is
> >>>>>> used in NMT. As NMT already collects stack traces before the
> >> CodeCache
> >>>> is
> >>>>>> initialized, contains() needs a check for this.
> >>>>>>
> >>>>>>  Wherever a row of platforms are listed, I sorted them alphabetically.
> >>>>>>
> >>>>>>  The jdk requires the file jvm.cfg.
> >>>>>>
> >>>>>> Please review. I please need a sponsor.
> >>>>>> http://cr.openjdk.java.net/~goetz/wr16/8166560-
> >>>> basic_s390/hotspot.wr01/
> >>>>>> http://cr.openjdk.java.net/~goetz/wr16/8166560-
> >> basic_s390/jdk.wr01/
> >>>>>>
> >>>>>> Best regards,
> >>>>>>   Goetz.
> >>>>>>
> >>>>>


More information about the hotspot-dev mailing list