RFR: 8208171: PPC64: Enrich SLP support
Michihiro Horie
HORIE at jp.ibm.com
Sun Sep 2 23:28:28 UTC 2018
Thanks a lot, Martin!
Best regards,
--
Michihiro,
IBM Research - Tokyo
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-dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>,
"ppc-aix-port-dev at openjdk.java.net"
<ppc-aix-port-dev at openjdk.java.net>, "Simonis, Volker"
<volker.simonis at sap.com>
Date: 2018/09/01 00:28
Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
Hi Michihiro,
thanks for the update. Looks correct, now.
I can also sponsor this change. Does anybody else want to review it?
Best regards,
Martin
From: Michihiro Horie <HORIE at jp.ibm.com>
Sent: Freitag, 31. August 2018 15:16
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-dev at openjdk.java.net;
ppc-aix-port-dev at openjdk.java.net; Simonis, Volker <volker.simonis at sap.com>
Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
Hi Martin,
Thank you so much for giving comments! I fixed version checks and
indentation.
New webrev:
http://cr.openjdk.java.net/~mhorie/8208171/webrev.02/
Best regards,
--
Michihiro,
IBM Research - Tokyo
Inactive hide details for "Doerr, Martin" ---2018/08/29 02:35:05---Hi
Michihiro, thank you for implementing it. I have just tak"Doerr, Martin"
---2018/08/29 02:35:05---Hi Michihiro, thank you for implementing it. I
have just taken a first look at your webrev.01.
From: "Doerr, Martin" <martin.doerr at sap.com>
To: Gustavo Romero <gromero at linux.vnet.ibm.com>, Michihiro Horie <
HORIE at jp.ibm.com>
Cc: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>, "
hotspot-dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>, "
ppc-aix-port-dev at openjdk.java.net" <ppc-aix-port-dev at openjdk.java.net>,
"Simonis, Volker" <volker.simonis at sap.com>
Date: 2018/08/29 02:35
Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
Hi Michihiro,
thank you for implementing it. I have just taken a first look at your
webrev.01.
It looks basically good. Only the Power version check seems to be
incorrect.
VM_Version::has_popcntb() checks for Power5.
I believe most instructions are available with Power7.
Some ones (vsubudm, ..., vmmuluwm, vpopcntw) were introduced with Power8?
We should check this carefully.
Also, indentation in register_ppc.hpp could get improved.
Thanks and best regard,
Martin
-----Original Message-----
From: Gustavo Romero <gromero at linux.vnet.ibm.com>
Sent: Donnerstag, 26. Juli 2018 16:02
To: Michihiro Horie <HORIE at jp.ibm.com>
Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
hotspot-dev at openjdk.java.net; Doerr, Martin <martin.doerr at sap.com>;
ppc-aix-port-dev at openjdk.java.net; Simonis, Volker <volker.simonis at sap.com>
Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
Hi Michi,
On 07/26/2018 01:43 AM, Michihiro Horie wrote:
> I updated webrev:
> http://cr.openjdk.java.net/~mhorie/8208171/webrev.01/
Thanks for providing an updated webrev and for fixing indentation and
function
order in assembler_ppc.inline.hpp as well. I have no further comments :)
Best Regards,
Gustavo
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
> Inactive hide details for Gustavo Romero ---2018/07/25 23:05:32---Hi
Michi, On 07/25/2018 02:43 AM, Michihiro Horie wrote:Gustavo Romero
---2018/07/25 23:05:32---Hi Michi, On 07/25/2018 02:43 AM, Michihiro Horie
wrote:
>
> From: Gustavo Romero <gromero at linux.vnet.ibm.com>
> To: Michihiro Horie/Japan/IBM at IBMJP, ppc-aix-port-dev at openjdk.java.net,
hotspot-dev at openjdk.java.net
> Cc: goetz.lindenmaier at sap.com, volker.simonis at sap.com, "Doerr, Martin" <
martin.doerr at sap.com>
> Date: 2018/07/25 23:05
> Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
>
>
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>
>
> Hi Michi,
>
> On 07/25/2018 02:43 AM, Michihiro Horie wrote:
> > Dear all,
> >
> > Would you review the following change?
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8208171
> > Webrev: http://cr.openjdk.java.net/~mhorie/8208171/webrev.00
> >
> > This change adds support for vectorized arithmetic calculation with
SLP.
> >
> > The to_vr function is added to convert VSR to VR. Currently, vecX is
associated with a VSR class vs_reg that only defines VSR32-51 in ppc.ad,
which are exactly overlapped with VRs. Instruction APIs receiving VRs use
the to_vr via vecX. Another thing is the change in sqrtF_reg to enable the
matching with SqrtVF. I think the change in sqrtF_reg would be fine due to
the ConvD2FNode::Value in convertnode.cpp.
>
> Looks good. Just a few comments:
>
> - In vmul4F_reg() would it be reasonable to use xvmulsp instead of
vmaddfp in
> order to avoid the splat?
>
> - Although all instructions added by your change where introduced in ISA
2.06,
> so POWER7 and above are OK, as I see probes for
PowerArchictecturePPC64=6|5 in
> vm_version_ppc.cpp (line 64), I'm wondering if there is any control
point to
> guarantee that these instructions won't be emitted on a CPU that does
not
> support them.
>
> - I think that in general string in format %{} are in upper case. For
instance,
> this the current output on optoassembly for vmul4F:
>
> 2941835 5b4 ADDI R24, R24, #64
> 2941836 5b8 vmaddfp VSR32,VSR32,VSR36 ! mul packed4F
> 2941837 5c0 STXVD2X [R17], VSR32 // store 16-byte Vector
>
> I think it would be better to be in upper case instead. I also think
that if
> the node match emits more than one instruction all instructions must
be listed
> in format %{}, since it's meant for detailed debugging. Finally I
think it
> would be better to replace \t! by \t// in that string (unless I'm
missing any
> special meaning for that char). So for vmul4F it would be something
like:
>
> 2941835 5b4 ADDI R24, R24, #64
> VSPLTISW VSR34, 0 // Splat 0 imm in
VSR34
> 2941836 5b8 VMADDFP VSR32,VSR32,VSR36,VSR34 // Mul packed4F
> 2941837 5c0 STXVD2X [R17], VSR32 // store 16-byte
Vector
>
>
> But feel free to change anything just after you get additional reviews :)
>
>
> > I confirmed this change with JTREG. In addition, I used attached micro
benchmarks.
> > /(See attached file: slp_microbench.zip)/
>
> Thanks for sharing it.
> Btw, another option to host it would be in the CR
> server, in http://cr.openjdk.java.net/~mhorie/8208171
>
>
> Best regards,
> Gustavo
>
> >
> > Best regards,
> > --
> > Michihiro,
> > IBM Research - Tokyo
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20180903/8e97ea91/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/ppc-aix-port-dev/attachments/20180903/8e97ea91/graycol.gif>
More information about the ppc-aix-port-dev
mailing list