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

Tue Sep 18 02:35:46 UTC 2018

Hi Gustavo,

Thanks a lot for your comments and review! I uploaded a latest webrev with
closing quotes and removing trailing spaces.

http://cr.openjdk.java.net/~mhorie/8210660/webrev.03/

Best regards,
--
Michihiro,
IBM Research - Tokyo

From:	Gustavo Romero <gromero at linux.vnet.ibm.com>
To:	Michihiro Horie/Japan/IBM at IBMJP, "Doerr, Martin"
            <martin.doerr at sap.com>
Cc:	"Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>,
            "hotspot-compiler-dev at openjdk.java.net"
            <hotspot-compiler-dev at openjdk.java.net>
Date:	2018/09/18 06:48
Subject:	Re: RFR(S): 8210660: PPC64: Mapping floating point registers to
            vsx registers in ppc.ad

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180918/fa78d1b8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180918/fa78d1b8/graycol.gif>