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

[Tobias Hartmann]

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