RFR(S): 8210660: PPC64: Mapping floating point registers to vsx registers in ppc.ad

[Gustavo Romero]

Hi Michi,

On 09/14/2018 08:42 AM, Michihiro Horie wrote:
> Hi Martin,
> 
> Thank you for your comment to improve this change and testing it. I uploaded a new webrev with format statements.<http://cr.openjdk.java.net/%7Emhorie/8210660/webrev.02/>
> http://cr.openjdk.java.net/~mhorie/8210660/webrev.02/ <http://cr.openjdk.java.net/%7Emhorie/8210660/webrev.02/>

Thanks for the updated webrev.

Only some nits:

Besides the missing closing quotes (") in MTVSRWZ and XXSPLTW format
strings I see trailing spaces in the following lines:

-    immI8  zero %{ (int)  0 %}
+    immI8  zero %{ (int)  0 %}
  
-    xscvdpspn_regF(tmpV, src);
+    xscvdpspn_regF(tmpV, src);

Curious enough, jcheck [1] is not complaining about them. I found it because I
set the color extension [2] in .hgrc:

[extensions]
color =

which marks trailing whitespaces in red.

I looks like some trailing spaces slipped also into related change
"8188139: PPC64: Superword Level Parallelization with VSX", in case you want to
fix them in a next change.

Finally, I think it would be better in XXPERMDI format string to replace
"Permute 16-byte register" to something like "Splat doubleword" to be like the
description in XXSPLTW that says "Splat word".
  
Otherwise, LGTM. Reviewed.

I'll sponsor that change.


Best regards,
Gustavo

[1] http://openjdk.java.net/projects/code-tools/jcheck/
[2] https://www.mercurial-scm.org/wiki/ColorExtension

> 
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
> 
> Inactive hide details for "Doerr, Martin" ---2018/09/14 17:30:04---Hi Michihiro, your webrev"Doerr, Martin" ---2018/09/14 17:30:04---Hi Michihiro, your webrev
> 
> From: "Doerr, Martin" <martin.doerr at sap.com>
> To: Michihiro Horie <HORIE at jp.ibm.com>
> Cc: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>, Gustavo Romero <gromero at linux.vnet.ibm.com>, "hotspot-compiler-dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.java.net>
> Date: 2018/09/14 17:30
> Subject: RE: RFR(S): 8210660: PPC64: Mapping floating point registers to vsx registers in ppc.ad
> 
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> 
> 
> 
> Hi Michihiro,
> 
> your webrev
> _http://cr.openjdk.java.net/~mhorie/8210660/webrev.01/_ <http://cr.openjdk.java.net/%7Emhorie/8210660/webrev.01/>
> looks good to me.
> 
> I only noticed that some instructs (e.g. for xscvdpspn and xxspltw) have no “format %{ … %}” specification so they are missing in the PrintOptoAssembly output. But this seems to be missing in the current version already.
> 
> We can test it while waiting for a 2nd review.
> 
> Thanks and best regards,
> Martin
> 
> 
> *From:* Michihiro Horie <HORIE at jp.ibm.com> *
> Sent:* Donnerstag, 13. September 2018 19:05*
> To:* Doerr, Martin <martin.doerr at sap.com>*
> Cc:* Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Gustavo Romero <gromero at linux.vnet.ibm.com>; hotspot-compiler-dev at openjdk.java.net*
> Subject:* RE: RFR(S): 8210660: PPC64: Mapping floating point registers to vsx registers in ppc.ad
> 
> Hi Martin,
> 
> Thank you so much for your review (and adding the ID in the subject :-).
> 
>  >I don’t think we need 2 nodes for ReplicateF with vector length 4. Both ones in your webrev are for Power8 so only one will be used.
> You're right, thanks. I removed a redundant one.
> 
> I also refactored ReplicateD with vector length 2.
> 
> Following is the latest webrev:_
> __http://cr.openjdk.java.net/~mhorie/8210660/webrev.01/_ <http://cr.openjdk.java.net/%7Emhorie/8210660/webrev.00/>
> 
> 
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
> 
> Inactive hide details for "Doerr, Martin" ---2018/09/13 16:25:33---Hi Michihiro, I have added "RFR(S): 8210660" to the subject."Doerr, Martin" ---2018/09/13 16:25:33---Hi Michihiro, I have added "RFR(S): 8210660" to the subject.
> 
> From: "Doerr, Martin" <_martin.doerr at sap.com_ <mailto:martin.doerr at sap.com>>
> To: Michihiro Horie <_HORIE at jp.ibm.com_ <mailto:HORIE at jp.ibm.com>>, "_hotspot-compiler-dev at openjdk.java.net_ <mailto:hotspot-compiler-dev at openjdk.java.net>" <_hotspot-compiler-dev at openjdk.java.net_ <mailto:hotspot-compiler-dev at openjdk.java.net>>
> Cc: Gustavo Romero <_gromero at linux.vnet.ibm.com_ <mailto:gromero at linux.vnet.ibm.com>>, "Lindenmaier, Goetz" <_goetz.lindenmaier at sap.com_ <mailto:goetz.lindenmaier at sap.com>>
> Date: 2018/09/13 16:25
> Subject: RE: RFR(S): 8210660: PPC64: Mapping floating point registers to vsx registers in ppc.ad
> 
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> 
> 
> 
> 
> Hi Michihiro,
> 
> I have added “RFR(S): 8210660” to the subject.
> 
> I don’t think we need 2 nodes for ReplicateF with vector length 4. Both ones in your webrev are for Power8 so only one will be used.
> Besides this, your change looks good to me.
> 
> Would you like to improve ReplicateD with vector length 2, too?
> 
> Thanks and best regards,
> Martin
> 
> *
> From:* Michihiro Horie <_HORIE at jp.ibm.com_ <mailto:HORIE at jp.ibm.com>> *
> Sent:* Mittwoch, 12. September 2018 18:11*
> To:* _hotspot-compiler-dev at openjdk.java.net_ <mailto:hotspot-compiler-dev at openjdk.java.net>*
> Cc:* Doerr, Martin <_martin.doerr at sap.com_ <mailto:martin.doerr at sap.com>>; Gustavo Romero <_gromero at linux.vnet.ibm.com_ <mailto:gromero at linux.vnet.ibm.com>>; Lindenmaier, Goetz <_goetz.lindenmaier at sap.com_ <mailto:goetz.lindenmaier at sap.com>>*
> Subject:* RFR: PPC64: Mapping floating point registers to vsx registers in ppc.ad
> 
> Dear all,
> 
> Would you please review the following change?
> 
> Bug: _https://bugs.openjdk.java.net/browse/JDK-8210660_
> Webrev: _http://cr.openjdk.java.net/~mhorie/8210660/webrev.00/_ <http://cr.openjdk.java.net/%7Emhorie/8210660/webrev.00/>
> 
> In the current code emit for replicating the floating point value in ppc.ad, a floating point value is once stored in order to load as an integer value. However, when SuperwordUseVSX is enabled, this is redundant because the floating point registers are overlapped with vsx registers 0-31. We can use vsx instructions for replicating the floating point value by mapping the floating point register to the vsx register.
> 
> 
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
> 
>