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

Wed Oct 19 14:59:47 UTC 2016

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] 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<mailto:volker.simonis at gmail.com>>
To: Michihiro Horie/Japan/IBM at IBMJP
Cc: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>>, hotspot compiler <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>, Hiroshi H Horii/Japan/IBM at IBMJP, Gustavo Romero <gromero at linux.vnet.ibm.com<mailto:gromero at linux.vnet.ibm.com>>, "Simonis, Volker" <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>, "ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>" <ppc-aix-port-dev at openjdk.java.net<mailto: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<mailto: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<mailto:goetz.lindenmaier at sap.com>>
> To: "Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>, Michihiro Horie/Japan/IBM at IBMJP,
> "Simonis, Volker" <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>,
> "ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>" <ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>>
> Cc: Hiroshi H Horii/Japan/IBM at IBMJP, Gustavo Romero
> <gromero at linux.vnet.ibm.com<mailto: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-
>> bounces at openjdk.java.net<mailto: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<mailto:HORIE at jp.ibm.com>>; Simonis, Volker
>> <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>; ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>
>> Cc: Hiroshi H Horii <HORII at jp.ibm.com<mailto:HORII at jp.ibm.com>>; Gustavo Romero
>> <gromero at linux.vnet.ibm.com<mailto: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]
>> Sent: Montag, 26. September 2016 09:37
>> To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>; Simonis, Volker
>> <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>; ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>
>> Cc: volker.simonis at gmail.com<mailto:volker.simonis at gmail.com>; Gustavo Romero
>> <gromero at linux.vnet.ibm.com<mailto:gromero at linux.vnet.ibm.com>>; Hiroshi H Horii <HORII at jp.ibm.com<mailto: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/hotspot-compiler-dev/attachments/20161019/79ee5dfc/attachment-0001.html>