RFR(M) JDK-8181809 PPC64: Leverage mtfprd/mffprd on POWER8
martin.doerr at sap.com
Mon Jun 26 12:47:45 UTC 2017
after some testing and reviewing the C1 part again, I found 2 bugs:
c1_LIRAssembler: is_stack() can't be used for this purpose as the value may be available in a register even though it was forced to stack. I just changed src_in_memory = !VM_Version::has_mtfprd() to make it consistent with LIRGenerator and removed the assertions which have become redundant.
c1_LIRGenerator: value.set_destroys_register() is still needed for conversion from FP to GP registers because they kill the src value by fctiwz/fctidz.
I just fixed these issues here in a copy of your webrev v2:
Please take a look and use this one for 2nd review.
From: Doerr, Martin
Sent: Montag, 26. Juni 2017 10:44
To: 'Matthew Brandyberry' <mbrandy at linux.vnet.ibm.com>; ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
Subject: RE: RFR(M) JDK-8181809 PPC64: Leverage mtfprd/mffprd on POWER8
you can run the pre-push check stand-alone: hg jcheck
I just had to add the commit message:
8181809: PPC64: Leverage mtfprd/mffprd on POWER8
Contributed-by: Matthew Brandyberry <mbrandy at linux.vnet.ibm.com>
(Note that the ':' after the bug id is important.)
and replace the Tabs the 2 C1 files to get it passing.
(I think that "Illegal tag name" warnings can be ignored.)
So only the copyright dates are missing which are not checked by jcheck.
But I don't need a new webrev if that's all which needs to be changed.
From: Matthew Brandyberry [mailto:mbrandy at linux.vnet.ibm.com]
Sent: Freitag, 23. Juni 2017 18:39
To: Doerr, Martin <martin.doerr at sap.com>; ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
Subject: Re: RFR(M) JDK-8181809 PPC64: Leverage mtfprd/mffprd on POWER8
Thanks Martin. Are there tools to help detect formatting errors like the
I'll keep an eye on this to see if I need to do anything else.
On 6/23/17 4:30 AM, Doerr, Martin wrote:
> Excellent. Thanks for the update. The C1 part looks good, too.
> Also, thanks for checking "I could not find evidence of any config that includes vpmsumb but not
> There are only a few formally required things:
> - The new C1 code contains Tab characters. It's not possible to push it without fixing this.
> - Copyright messages should be updated.
> - Minor resolution to get vm_version_ppc applied to recent jdk10/hs.
> If no other changes get requested, I can handle these issues this time before pushing.
> But we need another review, first.
> Thanks and best regards,
> -----Original Message-----
> From: Matthew Brandyberry [mailto:mbrandy at linux.vnet.ibm.com]
> Sent: Freitag, 23. Juni 2017 04:54
> To: Doerr, Martin <martin.doerr at sap.com>; ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
> Subject: Re: RFR(M) JDK-8181809 PPC64: Leverage mtfprd/mffprd on POWER8
> Updated webrev: http://cr.openjdk.java.net/~gromero/8181809/v2/
> See below for responses inline.
> On 6/20/17 8:38 AM, Matthew Brandyberry wrote:
>> Hi Martin,
>> Thanks for the review. I'll take a look at these areas and report
>> back -- especially the integration into C1.
>> On 6/20/17 8:33 AM, Doerr, Martin wrote:
>>> Hi Matt,
>>> thanks for providing this webrev. I had already thought about using
>>> these instructions for this purpose and your change matches pretty
>>> much what I'd do.
>>> Here a couple of comments:
>>> This was a lot of work. Thanks for doing it.
>>> effect(DEF dst, USE src); is redundant if a match rule match(Set dst
>>> (MoveL2D src)); exists.
>>> This part is in conflict with Michihiro's change which is already
>>> pushed in jdk10, but it's trivial to resolve. I'm ok with using
>>> has_vpmsumb() for has_mtfprd(). In the past, we sometimes had trouble
>>> with assuming that a certain Power processor supports all new
>>> instructions if it supports certain ones. We also use the hotspot
>>> code on as400 where certain instruction subsets were disabled while
>>> other Power 8 instructions were usable. Maybe you can double-check if
>>> there may exist configurations in which has_vpmsumb() doesn't match
> I could not find evidence of any config that includes vpmsumb but not
>>> It should also be possible to use the instructions in C1 compiler.
>>> Maybe you would like to take a look at it as well and see if it can
>>> be done with feasible effort.
>>> Here are some hints:
>>> The basic decisions are made in LIRGenerator::do_Convert. You could
>>> skip the force_to_spill or must_start_in_memory steps.
>>> The final assembly code gets emitted in LIR_Assembler::emit_opConvert
>>> where you could replace the store instructions.
>>> For testing, you can use -XX:TieredStopAtLevel=1, for example.
> Done. Please take a look.
>>> Thanks and best regards,
>>> -----Original Message-----
>>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>>> Behalf Of Matthew Brandyberry
>>> Sent: Montag, 19. Juni 2017 18:28
>>> To: ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
>>> Subject: RFR(M) JDK-8181809 PPC64: Leverage mtfprd/mffprd on POWER8
>>> This is a PPC-specific hotspot optimization that leverages the
>>> mtfprd/mffprd instructions for for movement between general purpose and
>>> floating point registers (rather than through memory). It yields a ~35%
>>> improvement measured via a microbenchmark. Please review: Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8181809 Webrev:
>>> http://cr.openjdk.java.net/~gromero/8181809/v1/ Thanks, Matt
More information about the hotspot-dev