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.


Reply via email to