[ping] RFR(M): 8160245: C1: Clean up platform #defines in c1_LIR.hpp.

Thu Jul 14 15:32:20 UTC 2016

Vladimir H. has to approve the request as the Hotspot Group lead.
The next step is the JDK9 Release Team which discusses FC requests
on Thursdays so it should be today... After the Release Team approves,
Vladimir will set the 'jdk9-fc-yes' label.

Dan

On 7/14/16 12:28 AM, Tobias Hartmann wrote:
> Hi Goetz,
>
> Vladimir K. already approved the request but I think he forgot to add the 'jdk9-fc-yes' label. Let's wait for him to do that.
>
> Best regards,
> Tobias
>
> On 14.07.2016 08:25, Lindenmaier, Goetz wrote:
>> Hi Tobias,
>>
>> don't we have to wait until the enhancement is accepted?
>> The bug still has tag jdk9-fc-request.
>> https://bugs.openjdk.java.net/browse/JDK-8160245
>>
>> Best regards,
>>    Goetz.
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
>>> Sent: Thursday, July 14, 2016 7:54 AM
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Vladimir Kozlov
>>> <vladimir.kozlov at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>>> Cc: hotspot-runtime-dev at openjdk.java.net runtime <hotspot-runtime-
>>> dev at openjdk.java.net>
>>> Subject: Re: [ping] RFR(M): 8160245: C1: Clean up platform #defines in
>>> c1_LIR.hpp.
>>>
>>> Hi,
>>>
>>> all tests passed. If everyone's fine with the latest webrev, I'll push this
>>> tomorrow:
>>> http://cr.openjdk.java.net/~thartmann/8160245/webrev.00/
>>>
>>> Goetz, could you please send me a changeset?
>>>
>>> Thanks,
>>> Tobias
>>>
>>> On 12.07.2016 15:07, Tobias Hartmann wrote:
>>>> Hi Goetz,
>>>>
>>>> On 12.07.2016 15:03, Lindenmaier, Goetz wrote:
>>>>> Hi Tobias,
>>>>>
>>>>> thanks for looking at this change!  Your additional edits look good.
>>>>> Thanks for spotting the problems.  Unfortunately, we don't have
>>>>> an aarch machine.
>>>> Sure, no problem! Actually, the 'fnoreg->encoding()' also showed up on
>>> SPARC.
>>>>> I also had problems with these asserts, as_FloatRegister(reg2)
>>>>> was not possible on ppc  because it did not like the '-1'. But that's
>>>>> fixed already.
>>>> Yes, I noticed that.
>>>>
>>>>> Good to know there is no PPC32 any more. I always had to take
>>>>> care I don't break anything in this 'invisible team mate' on Power :)
>>>>>
>>>>> Do you want me to make a new webrev with your edits? I'm
>>>>> fine with the webrev you mailed around, though.  I tested it to
>>>>> work on ppc, and I'll run it through our tests tonight.
>>>> You don't have to create a new webrev. If all testing passed and the other
>>> reviewers agree with the latest webrev, you can send me a changeset and I'll
>>> push it.
>>>> Best regards,
>>>> Tobias
>>>>
>>>>> Best regards,
>>>>>    Goetz.
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
>>>>>> Sent: Dienstag, 12. Juli 2016 14:34
>>>>>> To: Vladimir Kozlov <vladimir.kozlov at oracle.com>; Lindenmaier, Goetz
>>>>>> <goetz.lindenmaier at sap.com>; hotspot-compiler-
>>> dev at openjdk.java.net
>>>>>> Cc: hotspot-runtime-dev at openjdk.java.net runtime <hotspot-runtime-
>>>>>> dev at openjdk.java.net>
>>>>>> Subject: Re: [ping] RFR(M): 8160245: C1: Clean up platform #defines in
>>>>>> c1_LIR.hpp.
>>>>>>
>>>>>> Hi Goetz,
>>>>>>
>>>>>> On 06.07.2016 00:47, Vladimir Kozlov wrote:
>>>>>>> CC to runtime too since it has changes in Interpreter and affects our
>>> closed
>>>>>> code.
>>>>>>> Hi Goetz,
>>>>>>>
>>>>>>> Please, set "Fix Version".
>>>>>>>
>>>>>>> And if it is JDK 9 you need FC Extension Request since you converted it
>>> to
>>>>>> Enhancement:
>>>>>>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-
>>> June/004443.html
>>>>>>> Someone from Oracle have to sponsor it and do related closed code
>>>>>> changes and testing.
>>>>>>
>>>>>> webrev.03 looks good to me. I did the necessary closed changes (review
>>> is
>>>>>> pending) and also adapted some of the public code.
>>>>>>
>>>>>> Incremental webrev:
>>>>>> http://cr.openjdk.java.net/~thartmann/8160245/webrev.incremental/
>>>>>>
>>>>>> Full webrev:
>>>>>> http://cr.openjdk.java.net/~thartmann/8160245/webrev.00/
>>>>>>
>>>>>> I removed the PPC code because since 8u33, SE Embedded releases no
>>>>>> longer include 32-bit PPC platforms (see [1], [2]) and moved the ARM
>>> specific
>>>>>> code into the closed sources. I adapted the copyright dates and
>>> refactored
>>>>>> some related code.
>>>>>>
>>>>>> Also, 'fnoreg->encoding()' fails because RegisterImpl::encoding() checks
>>> for
>>>>>> is_valid() which is false for fnoreg. We should check for
>>>>>> 'as_FloatRegister(reg2) != fnoreg'.
>>>>>>
>>>>>> Tests are running.
>>>>>>
>>>>>> Best regards,
>>>>>> Tobias
>>>>>>
>>>>>> [1] http://www.oracle.com/technetwork/java/embedded/embedded-
>>>>>> se/downloads/index.html
>>>>>> [2] http://www.oracle.com/technetwork/java/javase/certconfig-
>>>>>> 2095354.html
>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 7/5/16 3:47 AM, Lindenmaier, Goetz wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> could someone please have a look at this issue? I recap the
>>> description
>>>>>>>> below, and I also updated the webrev which would no more apply to
>>>>>>>> hs-comp:
>>>>>>>>
>>>>>>>> c1_LIR.hpp defines a row of functions guarded by platform
>>>>>>>> defines. This is bad style and hinders new platform ports.
>>>>>>>> (I'm working on S390 aka Z :))
>>>>>>>>
>>>>>>>> This change removes the majority of these defines. It introduces
>>>>>>>> common headers, and moves implementations to c1_LIR_<cpu>.cpp
>>> files.
>>>>>>>> It guards single_softfp() and double_softfp() by __SOFTFP__.
>>>>>>>> This is not used in any openJdk platform. I can not test this
>>>>>>>> on the closed platforms ARM32 and PPC32.
>>>>>>>>
>>>>>>>> It removes the guard around the LIR_Address constructor. There
>>>>>>>> is no point in guarding this code, verify() assures by
>>>>>>>> assertions that it can not be misused. I also introduce a new
>>>>>>>> constructor that leaves out the scale argument and introduce
>>>>>>>> some usages on X86.
>>>>>>>>
>>>>>>>> This change also moves verify() to the new platform files. In the
>>>>>>>> header, LIR_ADDRESS_PD_VERIFY was used to guard usage
>>>>>>>> of pd_verify(). Neither of these are used in openJdk. If this define
>>>>>>>> is used in the closed ports pd_verify() must be renamed to verify().
>>>>>>>>
>>>>>>>> The code that was previously guarded by ARM, ARM32 or PPC32 is
>>>>>>>> moved to a properly guarded section in c1_LIR.cpp. Actually,
>>>>>>>> it should be moved to according new files c1_LIR_<cpu>.cpp in
>>>>>>>> the closed ports. But this way the change should basically
>>>>>>>> work for the closed ports.
>>>>>>>>
>>>>>>>> I added fnoreg on x86 to note down the code similarly on all
>>>>>>>> platforms.
>>>>>>>>
>>>>>>>> I cleaned up a flag with a limited range on PPC_32.
>>>>>>>>
>>>>>>>> generate_stack_overflow_check() in
>>> templateInterpreterGenerator.hpp
>>>>>>>> is defined with different parameters for the platforms. I added default
>>>>>>>> parameters noreg so that the signature is the same for all platforms.
>>>>>>>>
>>>>>>>> Please review this change. I please need a sponsor.
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8160245
>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8160245-
>>> simplifyC1/webrev.02/
>>>>>>>> I built and tested this on linuxx86_64, solaris_sparc and
>>>>>>>> the ppc platforms.
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>>    Goetz.
>>>>>>>>
>>>>>>>>