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

[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.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.
>>>>
>>>