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

[Lindenmaier, Goetz]

Hi Coleen, 

I edited the change according to your comments and made a new webrev:
http://cr.openjdk.java.net/~goetz/wr16/8160245-simplifyC1/webrev.03/

Best regards,
  Goetz.

> -----Original Message-----
> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
> Sent: Mittwoch, 6. Juli 2016 21:31
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: [ping] RFR(M): 8160245: C1: Clean up platform #defines in
> c1_LIR.hpp.
> 
> 
> 
> On 7/6/16 3:02 PM, Lindenmaier, Goetz wrote:
> > Hi Coleen,
> >
> > You're right, I only need two args.  That's wrong because I edited
> > it before 8159335 was merged to hs-comp which removed an
> > argument.  I'll fix that.
> >
> > For the overloaded functions, do you want me to add
> > an implementation with ShouldNotReachHere() for the
> > unused versions?  And isn't that basically the same as having
> > unused parameters?
> 
> I don't think it's necessary to have a ShouldNotReachHere for the unused
> versions.   If you call the one with no parameters on a platform with
> the two parameter implementation, you'll get an undefined reference at
> linktime, which is better than a crash.
> 
> With the noreg default parameter versions, you could make the mistake of
> not passing any registers or the wrong number of registers, and I guess
> the result would be a crash at runtime but that can be hard to
> understand why it happened.
> 
> So I would prefer both declarations without default arguments, and each
> platform code calls the right version and doesn't call the other.
> 
> Coleen
> 
> >
> > Best regards,
> >    Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> >> bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
> >> Sent: Wednesday, July 06, 2016 1:24 AM
> >> To: hotspot-runtime-dev at openjdk.java.net
> >> Subject: Re: [ping] RFR(M): 8160245: C1: Clean up platform #defines in
> >> c1_LIR.hpp.
> >>
> >>
> >> Why is there a third register parameter to
> >> generate_stack_overflow_check() that is unused on all the platforms?
> >>
> >> I would almost rather have one version with no arguments overloaded
> with
> >> one with two, than three default parameters.  It seems dangerous.
> >>
> >> thanks,
> >> Coleen
> >>
> >> On 7/5/16 6:47 PM, 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.
> >>>
> >>> 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.
> >>>>
> >>>>