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

[Matthew Brandyberry]

Thanks Martin.  That looks good.  Is there anything I need to do to 
request a 2nd review?

On 6/26/17 7:47 AM, Doerr, Martin wrote:
> 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
>>>>
>>>>