RFR:8166684:implement intrinsic code with vector instructions for Unsafe.copyMemory()

Fri Oct 21 07:30:48 UTC 2016

Hi Martin,

looks good!

Thanks,
Volker

On Thu, Oct 20, 2016 at 7:12 PM, Doerr, Martin <martin.doerr at sap.com> wrote:

> Hi Volker and Michihiro,
>
>
>
> here’s the updated version of the webrev:
>
> http://cr.openjdk.java.net/~mdoerr/8166684_PPC64_unsafe_
> copymemory/webrev.02/
>
>
>
> We’ll push this version if you agree.
>
>
>
> Best regards,
>
> Martin
>
>
>
> *From:* Volker Simonis [mailto:volker.simonis at gmail.com]
> *Sent:* Donnerstag, 20. Oktober 2016 18:50
> *To:* Michihiro Horie
> *Cc:* Doerr, Martin; hotspot-compiler-dev at openjdk.java.net; Hiroshi H
> Horii; Simonis, Volker; ppc-aix-port-dev at openjdk.java.net
>
> *Subject:* Re: RE: RFR:8166684:implement intrinsic code with vector
> instructions for Unsafe.copyMemory()
>
>
>
> Hi Michihiro,
>
> thanks for doing the cleanup. The change looks good now.
>
> The only thing I noticed is that you used:
>
> 1759       __ li(tmp3, 0);
>
> 1782         __ lwzx(tmp2, R3_ARG1, tmp3);
>
> 1783         __ stwx(tmp2, R4_ARG2, tmp3);
>
>
>
> This can be simplified by using either the two-argument version of lwzx or the lwz with zero immediate.
>
> Also some of the comments are not aligned anymore, after you've removed the now obsolete '0' arguments.
>
> But there's no need to do a new webrev. If you agree, we will add this two cosmetic changes before pushing.
>
> Thanks once again and best regards,
> Volker
>
>
>
>
>
>
>
> On Wed, Oct 19, 2016 at 5:16 PM, Michihiro Horie <HORIE at jp.ibm.com> wrote:
>
> Hi Martin,
>
> >Please also test the debug build in the future.
> >You forgot one usage of lxvd2x in VM_Version::determine_features().
> >I fixed that locally and I will test it.
> Thanks a lot for your advice and help!
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
> [image: Inactive hide details for "Doerr, Martin" ---2016/10/20
> 00:00:59---Hi Michihiro, thanks for the new webrev. Please also test th]"Doerr,
> Martin" ---2016/10/20 00:00:59---Hi Michihiro, thanks for the new webrev.
> Please also test the debug build in the future.
>
> From: "Doerr, Martin" <martin.doerr at sap.com>
> To: Michihiro Horie/Japan/IBM at IBMJP, "volker.simonis at gmail.com" <
> volker.simonis at gmail.com>
> Cc: "hotspot-compiler-dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.
> java.net>, Hiroshi H Horii/Japan/IBM at IBMJP, "Simonis, Volker" <
> volker.simonis at sap.com>, "ppc-aix-port-dev at openjdk.java.net" <
> ppc-aix-port-dev at openjdk.java.net>
> Date: 2016/10/20 00:00
> Subject: RE: RE: RFR:8166684:implement intrinsic code with vector
> instructions for Unsafe.copyMemory()
> ------------------------------
>
>
>
>
> Hi Michihiro,
>
> thanks for the new webrev. Please also test the debug build in the future.
> You forgot one usage of lxvd2x in VM_Version::determine_features().
> I fixed that locally and I will test it.
>
> Besides that, it looks good to me.
>
> I think I can push it if Volker is ok with it (no need for new webrev).
>
> Best regards,
> Martin
>
>
> *From:* hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> bounces at openjdk.java.net <hotspot-compiler-dev-bounces at openjdk.java.net>] *On
> Behalf Of *Michihiro Horie
> * Sent:* Dienstag, 18. Oktober 2016 17:31
> * To:* volker.simonis at gmail.com
> * Cc:* hotspot-compiler-dev at openjdk.java.net; Hiroshi H Horii; Simonis,
> Volker; ppc-aix-port-dev at openjdk.java.net
> * Subject:* Re: RE: RFR:8166684:implement intrinsic code with vector
> instructions for Unsafe.copyMemory()
>
> Hi Volker,
>
> Thanks for your helpful comments, I fixed the code.
>
> >And finally, "enerate_disjoint_int_copy_core" tries to achieve 8-byte
> >alignment if possible. Would that make sense here as well? And what
> >are the alignement requirements of the vector instruction - can they
> >cope with 4-byte aligned data?
> It makes sense and I introduced the code with trying to achieve 8-byte
> alignment if possible.
> The 4-byte aligned data can be also processed just as
> "generate_disjoint_int_copy_core" handles the 4-byte aligned data with
> VSX.
>
> JIRA:https://bugs.openjdk.java.net/browse/JDK-8166684
> Webrev: http://cr.openjdk.java.net/~horii/8166684/webrev.02/
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
>
> ----- Original message -----
> From: Volker Simonis <volker.simonis at gmail.com>
> To: Michihiro Horie/Japan/IBM at IBMJP
> Cc: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>, hotspot compiler <
> hotspot-compiler-dev at openjdk.java.net>, Hiroshi H Horii/Japan/IBM at IBMJP,
> Gustavo Romero <gromero at linux.vnet.ibm.com>, "Simonis, Volker" <
> volker.simonis at sap.com>, "ppc-aix-port-dev at openjdk.java.net" <
> ppc-aix-port-dev at openjdk.java.net>
> Subject: Re: RE: RFR:8166684:implement intrinsic code with vector
> instructions for Unsafe.copyMemory()
> Date: Tue, Oct 11, 2016 6:55 PM
>
> Hi Michihiro,
>
> thanks for your contribution and sorry for the late reply.
>
> I have some comments regarding your change:
>
> lxvd2x(tmp_vsr1, 0, R3_ARG1);
> stxvd2x(tmp_vsr1, 0, R4_ARG2);
>
>
> "lxvd2x" the second argument has to be a register not a constant. Your
> code works because '0' is implicitly converted to register R0 which in
> turn encodes as zero in the instruction. But that's not nice from a
> software engineering perspective :)
>
> Please create instead a new version of "lxvd2x" which only takes two
> arguments (one vector register and one general purpose register) and
> explicitly encodes the zero in the instructions. Please also add an
> assertion to the three-argument version of lxvd2x which asserts that
> the second argument is not R0. You can use "ra0mem" for that (just
> have a look at the two versions of "ldx" for an example).
>
> The same applies to "stxvd2x". And can you please also fix the other
> occurrences of lxvd2x/stxvd2x which you've previously introduced with
> change 8158232.
>
> With that change ("8158232: PPC64: improve byte, int and long array
> copy stubs by using VSX instructions") you've also introduced
> prefetching when using the vector instructions and you aligned the
> backbranch target to 32 byte (see "enerate_disjoint_int_copy_core").
> Wouldn't that be useful here as well?
>
> And finally, "enerate_disjoint_int_copy_core" tries to achieve 8-byte
> alignment if possible. Would that make sense here as well? And what
> are the alignement requirements of the vector instruction - can they
> cope with 4-byte aligned data?
>
> Thank you and best regards,
> Volker
>
>
>
>
> On Fri, Sep 30, 2016 at 3:30 PM, Michihiro Horie <HORIE at jp.ibm.com> wrote:
> > Hi Goetz, Martin,
> >
> > Would you review this? The initialization of tmp1 is now outside the
> loop.
> >
> > JIRA: https://bugs.openjdk.java.net/browse/JDK-8166684
> > Webrev: http://cr.openjdk.java.net/~horii/8166684/webrev.01/
> >
> > We created webrev by ourselves, and cced to hotspot-compiler-dev.
> >
> > Best regards,
> > --
> > Michihiro,
> > IBM Research - Tokyo
> >
> >
> >
> > ----- Original message -----
> > From: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>
> > To: "Doerr, Martin" <martin.doerr at sap.com>, Michihiro
> Horie/Japan/IBM at IBMJP,
> > "Simonis, Volker" <volker.simonis at sap.com>,
> > "ppc-aix-port-dev at openjdk.java.net" <ppc-aix-port-dev at openjdk.java.net>
> > Cc: Hiroshi H Horii/Japan/IBM at IBMJP, Gustavo Romero
> > <gromero at linux.vnet.ibm.com>
> > Subject: RE: RFR:8166684:implement intrinsic code with vector
> instructions
> > for Unsafe.copyMemory()
> > Date: Mon, Sep 26, 2016 10:51 PM
> >
> > Hi,
> >
> > please post this RFR also to hotspot-compiler-dev. It must be reviewed
> > on one of the official lists before it can be pushed. Ppc-aix-port-dev
> > is only for communication about the port, not for reviews.
> >
> > Also I would appreciate if you could upload your webrevs yourselves.
> > We are happy to help out in the beginning, and also with testing,
> > reviewing and pushing, but making webrevs is a task I don't see on
> > our side in the long term.
> >
> > Thanks and best regards,
> > Goetz.
> >
> >> -----Original Message-----
> >> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev- <ppc-aix-port-dev->
> >> bounces at openjdk.java.net] On Behalf Of Doerr, Martin
> >> Sent: Montag, 26. September 2016 11:53
> >> To: Michihiro Horie <HORIE at jp.ibm.com>; Simonis, Volker
> >> <volker.simonis at sap.com>; ppc-aix-port-dev at openjdk.java.net
> >> Cc: Hiroshi H Horii <HORII at jp.ibm.com>; Gustavo Romero
> >> <gromero at linux.vnet.ibm.com>
> >> Subject: RE: RFR:8166684:implement intrinsic code with vector
> instructions
> >> for Unsafe.copyMemory()
> >>
> >> Hi Michihiro,
> >>
> >>
> >>
> >> the initialization of tmp1 should be done outside of the loop. Beside
> >> that, the
> >> change looks good:
> >>
> >> http://cr.openjdk.java.net/~mdoerr/8166684_PPC64_unsafe_copymemory/
> >> webrev.00/
> >> <http://cr.openjdk.java.net/~mdoerr/8166684_PPC64_unsafe_copymemory
> >> /webrev.00/>
> >>
> >>
> >>
> >> Best regards,
> >>
> >> Martin
> >>
> >>
> >>
> >>
> >>
> >> From: Michihiro Horie [mailto:HORIE at jp.ibm.com <HORIE at jp.ibm.com>]
> >> Sent: Montag, 26. September 2016 09:37
> >> To: Doerr, Martin <martin.doerr at sap.com>; Simonis, Volker
> >> <volker.simonis at sap.com>; ppc-aix-port-dev at openjdk.java.net
> >> Cc: volker.simonis at gmail.com; Gustavo Romero
> >> <gromero at linux.vnet.ibm.com>; Hiroshi H Horii <HORII at jp.ibm.com>
> >> Subject: RFR:8166684:implement intrinsic code with vector instructions
> for
> >> Unsafe.copyMemory()
> >>
> >>
> >>
> >> Dear all,
> >>
> >> Could I please request reviews for the following change?
> >> This change was created for JDK9.
> >>
> >> I added fixes to the intrinsic code for sun.misc.Unsafe.copyMemory() by
> >> using VSX.
> >> Since Spark often invokes Unsafe.copyMemory(), it is beneficial to use
> the
> >> vector instructions for these intrinsic code.
> >>
> >> jira: https://bugs.openjdk.java.net/browse/JDK-8166684
> >> <https://bugs.openjdk.java.net/browse/JDK-8166684>
> >> diff: (See attached file: unsafe-copymemory-openjdk9.diff)
> >>
> >> Best regards,
> >> --
> >> Michihiro Horie,
> >> IBM Research - Tokyo
> >
> >
> >
> >
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20161021/f1c22920/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20161021/f1c22920/image001-0001.gif>