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.hol...@oracle.com] Sent: Dienstag, 4. Oktober 2016 09:56 To: Lindenmaier, Goetz ; Volker Simonis Cc: hotspot-...@openjdk.java.net; core-libs-dev 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 Cc: Lindenmaier, Goetz ; hotspot- d...@openjdk.java.net; core-libs-dev Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port. On Fri, Sep 23, 2016 at 8:11 AM, David Holmes 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.
RE: RFR(M): 8166560: [s390] Basic enablement of s390 port.
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 ; Volker Simonis > > Cc: hotspot-...@openjdk.java.net; core-libs-dev 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 > >> Cc: Lindenmaier, Goetz ; hotspot- > >> d...@openjdk.java.net; core-libs-dev > >> Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port. > >> > >> On Fri, Sep 23, 2016 at 8:11 AM, David Holmes > > >> 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. > >>>> > >>>
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 Cc: Lindenmaier, Goetz ; hotspot- d...@openjdk.java.net; core-libs-dev Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port. On Fri, Sep 23, 2016 at 8:11 AM, David Holmes 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.
RE: RFR(M): 8166560: [s390] Basic enablement of s390 port.
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. 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 ... 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 > Cc: Lindenmaier, Goetz ; hotspot- > d...@openjdk.java.net; core-libs-dev > Subject: Re: RFR(M): 8166560: [s390] Basic enablement of s390 port. > > On Fri, Sep 23, 2016 at 8:11 AM, David Holmes > 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. > >> > >
Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
On Fri, Sep 23, 2016 at 8:11 AM, David Holmes 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. >> >
Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
Hi Alan, I've created a JEP. You can find it here: https://bugs.openjdk.java.net/browse/JDK-8166730 I've also just sent an RFR for the JEP to the various list with some more detailed information. It would be great if you could review it for the class library. The changes to the jdk-repo are really minimal - just adding a jvm.cfg file for s390x. Thank you and best regards, Volker On Sat, Sep 24, 2016 at 12:08 PM, Alan Bateman wrote: > On 23/09/2016 06:52, 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. >> >> > Out of curiosity, is there is JEP for the Linux/S390 port? There were JEPs > for the Linux/AArch64 and PowerPC/AIX ports and just wondering if there is > one coming for the S390 port too? > > -Alan
Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
On 23/09/2016 06:52, 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. Out of curiosity, is there is JEP for the Linux/S390 port? There were JEPs for the Linux/AArch64 and PowerPC/AIX ports and just wondering if there is one coming for the S390 port too? -Alan
Re: RFR(M): 8166560: [s390] Basic enablement of s390 port.
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 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.