[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: [ping] RFR(M): 8160245: C1: Clean up platform #defines in c1_LIR.hpp.
From: Tobias Hartmann <tobias.hartmann () oracle ! com>
Date: 2016-07-14 15:35:32
Message-ID: 5787B144.7080001 () oracle ! com
[Download RAW message or body]
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@oracle.com]
> > > > Sent: Thursday, July 14, 2016 7:54 AM
> > > > To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; Vladimir Kozlov
> > > > <vladimir.kozlov@oracle.com>; hotspot-compiler-dev@openjdk.java.net
> > > > Cc: hotspot-runtime-dev@openjdk.java.net runtime <hotspot-runtime-
> > > > dev@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@oracle.com]
> > > > > > > Sent: Dienstag, 12. Juli 2016 14:34
> > > > > > > To: Vladimir Kozlov <vladimir.kozlov@oracle.com>; Lindenmaier, Goetz
> > > > > > > <goetz.lindenmaier@sap.com>; hotspot-compiler-
> > > > dev@openjdk.java.net
> > > > > > > Cc: hotspot-runtime-dev@openjdk.java.net runtime <hotspot-runtime-
> > > > > > > dev@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.
> > > > > > > > >
> > > > > > > > >
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic