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

Thu Oct 20 16:49:57 UTC 2016

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*
> <https://bugs.openjdk.java.net/browse/JDK-8166684>
> Webrev: *http://cr.openjdk.java.net/~horii/8166684/webrev.02/*
> <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*
> <volker.simonis at gmail.com>>
> To: Michihiro Horie/Japan/IBM at IBMJP
> Cc: "Lindenmaier, Goetz" <*goetz.lindenmaier at sap.com*
> <goetz.lindenmaier at sap.com>>, hotspot compiler <
> *hotspot-compiler-dev at openjdk.java.net*
> <hotspot-compiler-dev at openjdk.java.net>>, Hiroshi H Horii/Japan/IBM at IBMJP,
> Gustavo Romero <*gromero at linux.vnet.ibm.com* <gromero at linux.vnet.ibm.com>>,
> "Simonis, Volker" <*volker.simonis at sap.com* <volker.simonis at sap.com>>, "
> *ppc-aix-port-dev at openjdk.java.net* <ppc-aix-port-dev at openjdk.java.net>" <
> *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*
> <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*
> <https://bugs.openjdk.java.net/browse/JDK-8166684>
> > Webrev: *http://cr.openjdk.java.net/~horii/8166684/webrev.01*
> <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*
> <goetz.lindenmaier at sap.com>>
> > To: "Doerr, Martin" <*martin.doerr at sap.com* <martin.doerr at sap.com>>,
> Michihiro Horie/Japan/IBM at IBMJP,
> > "Simonis, Volker" <*volker.simonis at sap.com* <volker.simonis at sap.com>>,
> > "*ppc-aix-port-dev at openjdk.java.net* <ppc-aix-port-dev at openjdk.java.net>"
> <*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* <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* <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* <HORIE at jp.ibm.com>>; Simonis,
> Volker
> >> <*volker.simonis at sap.com* <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 <*HORII at jp.ibm.com* <HORII at jp.ibm.com>>; Gustavo
> Romero
> >> <*gromero at linux.vnet.ibm.com* <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*
> <http://cr.openjdk.java.net/~mdoerr/8166684_PPC64_unsafe_copymemory>/
> >> webrev.00/
> >> <*http://cr.openjdk.java.net/~mdoerr/8166684_PPC64_unsafe_copymemory*
> <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* <martin.doerr at sap.com>>;
> Simonis, Volker
> >> <*volker.simonis at sap.com* <volker.simonis at sap.com>>;
> *ppc-aix-port-dev at openjdk.java.net* <ppc-aix-port-dev at openjdk.java.net>
> >> Cc: *volker.simonis at gmail.com* <volker.simonis at gmail.com>; Gustavo
> Romero
> >> <*gromero at linux.vnet.ibm.com* <gromero at linux.vnet.ibm.com>>; Hiroshi H
> Horii <*HORII at jp.ibm.com* <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>
> >> <*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/20161020/921a1311/attachment-0001.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/20161020/921a1311/graycol-0001.gif>