RFR(M) JDK-8181809 PPC64: Leverage mtfprd/mffprd on POWER8

[Doerr, Martin]

Hi Matt,

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:
http://cr.openjdk.java.net/~mdoerr/8181809_ppc64_mtfprd/v2/

Please take a look and use this one for 2nd review.

Best regards,
Martin


-----Original Message-----
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

Hi Matt,

you can run the pre-push check stand-alone: hg jcheck
See:
http://openjdk.java.net/projects/code-tools/jcheck/

I just had to add the commit message:
8181809: PPC64: Leverage mtfprd/mffprd on POWER8
Reviewed-by: mdoerr
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.

Best regards,
Martin


-----Original Message-----
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 
tab characters?

I'll keep an eye on this to see if I need to do anything else.

-Matt

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
> mtfprd."
>
> 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,
> Martin
>
>
> -----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:
>>> ppc.ad:
>>> 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.
> Fixed.
>>> vm_version:
>>> 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
>>> has_mtfprd().
> I could not find evidence of any config that includes vpmsumb but not
> mtfprd.
>>> C1:
>>> 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,
>>> Martin
>>>
>>>
>>> -----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
>>>
>>>