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? Best regards, Goetz. > -----Original Message----- > From: David Holmes [mailto:david.hol...@oracle.com] > Sent: Dienstag, 4. Oktober 2016 09:56 > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Volker Simonis > <volker.simo...@gmail.com> > Cc: hotspot-...@openjdk.java.net; core-libs-dev <core-libs- > d...@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.simo...@gmail.com] > >> Sent: Dienstag, 27. September 2016 19:58 > >> To: David Holmes <david.hol...@oracle.com> > >> Cc: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; hotspot- > >> d...@openjdk.java.net; core-libs-dev <core-libs-dev@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.hol...@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. > >>>> > >>>