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

[Lindenmaier, Goetz]

Hi Tobias,

here is a final webrev including a jchecked changeset:
http://cr.openjdk.java.net/~goetz/wr16/8160245-simplifyC1/webrev.04/

Thanks a lot for your help!

Best regards,
 Goetz.

> -----Original Message-----
> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
> Sent: Donnerstag, 14. Juli 2016 08:28
> 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 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.
> >>>>>>>
> >>>>>>>