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

David Holmes david.holmes at oracle.com
Mon Oct 10 06:47:20 UTC 2016


Thanks Goetz!

David

On 10/10/2016 4:37 PM, Lindenmaier, Goetz wrote:
> Hi David,
>
> ok, I removed the comment ...
> http://cr.openjdk.java.net/~goetz/wr16/8166560-basic_s390/hotspot.wr04/
>
> Best regards,
>   Goetz.
>
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Mittwoch, 5. Oktober 2016 04:17
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Volker Simonis
>> <volker.simonis at gmail.com>
>> Cc: 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 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.holmes at oracle.com]
>>>> Sent: Dienstag, 4. Oktober 2016 09:56
>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Volker Simonis
>>>> <volker.simonis at gmail.com>
>>>> Cc: 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.
>>>>
>>>> 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.
>>>>>>>>
>>>>>>>


More information about the hotspot-dev mailing list