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

2016-10-04 Thread David Holmes 

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 <goetz.lindenma...@sap.com>; Volker Simonis
<volker.simo...@gmail.com>
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 <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.

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

2016-10-04 Thread Lindenmaier, Goetz 
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  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.
> >>>>
> >>>

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

2016-10-04 Thread David Holmes 

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.

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

2016-09-28 Thread Lindenmaier, Goetz 
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 <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.
> >>
> >

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

2016-09-27 Thread Volker Simonis 
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.

2016-09-27 Thread Volker Simonis 
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.

2016-09-24 Thread Alan Bateman 

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.

2016-09-23 Thread David Holmes 

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.

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

2016-09-22 Thread Lindenmaier, Goetz 
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.