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

Thu Jul 14 15:35:32 UTC 2016

Hi Daniel,

okay, thanks for the clarification!

Best regards,
Tobias

On 14.07.2016 17:32, Daniel D. Daugherty wrote:
> 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.
>>>>>>>>>
>>>>>>>>>
> 