RFR: 8208171: PPC64: Enrich SLP support
Doerr, Martin
martin.doerr at sap.com
Tue Sep 4 16:20:58 UTC 2018
Hi Michihiro,
thanks for looking into the problems.
I also prefer "vnoreg" and "vsnoreg".
I'd be fine with just adding "&& SuperwordUseVSX" for the new rules in "match_rule_supported".
Can you reproduce the test failures?
The very same VM works fine on a different Power8 machine which uses the same instructions by C2.
The VM was built on the machine where it works ("SUSE Linux Enterprise Server 12 SP1").
I have seen several linux kernel changes regarding saving and restoring the VSX registers.
I still haven't found out how the kernel determines things like "tsk->thread.used_vsr" which is used to set "msr |= MSR_VEC".
Maybe something is missing which tells the kernel that we're using it. But that's just a guess.
Best regards,
Martin
From: Michihiro Horie <HORIE at jp.ibm.com>
Sent: Dienstag, 4. September 2018 07:32
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 <hotspot-compiler-dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
Hi Goetz, Martin, and Gustavo,
>First, this should have been reviewed on hotspot-compiler-dev. It is clearly
>a compiler change.
>https://urldefense.proofpoint.com/v2/url?u=http-3A__mail.openjdk.java.net_mailman_listinfo&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=AwJriSOfe9Z0niOEpp6HGgsCBhKwnM19dyn4CipYwyU&s=O9RJz8qw_uJHSJyEdWsuR2j_lgnquX3sbwyEgkFZ3YQ&e=<http://mail.openjdk.java.net/mailman/listinfo> says that hotspot-dev is for
>"Technical discussion about the development of the HotSpot virtual machine that's not specific to any particular component"
>while hotspot-compiler-dev is for
>"Technical discussion about the development of the HotSpot bytecode compilers"
I understood the instruction and would use hotspot-compiler-dev in future RFRs, thanks.
> Why do you rename vnoreg to vnoregi?
I followed the way of coding for vsnoreg and vsnoregi, but the renaming does not look necessary. I would get this part back. Should I also rename vsnoregi to vsnoreg?
>we noticed jtreg test failures when using this change:
>compiler/runtime/safepoints/TestRegisterRestoring.java
>compiler/runtime/Test7196199.java
>
>TestRegisterRestoring is a simple test which returns arbitrary results instead of 10000.
>
>We didn't see it on all machines, so it might be an issue with saving&restoring VR registers in the signal handler.
>The machine which I have used has "SUSE Linux Enterprise Server 12 SP3" with kernel 4.4.126-94.22-default.
Thank you for letting me know the issue, I will try to reproduce this on a SUSE machine.
>I also noticed that "-XX:-SuperwordUseVSX" crashes with bad ad file when your patch is applied. Looks like matching the vector nodes needs to be prevented.
Thank you for pointing out another issue. Currently I do not hit this problem, but preventing to match the vector nodes makes sense to avoid the crash. I did not prepare match rules for non-vector nodes, so it might be better to prepare them similarly like the Replicate* rules, in any case.
Gustavo, thanks for the wrap-up!
Best regards,
--
Michihiro,
IBM Research - Tokyo
[Inactive hide details for "Doerr, Martin" ---2018/09/04 02:18:24---Hi Gustavo and Michihiro, we noticed jtreg test failures whe]"Doerr, Martin" ---2018/09/04 02:18:24---Hi Gustavo and Michihiro, we noticed jtreg test failures when using this change:
From: "Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
To: 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>>, Michihiro Horie <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>, "hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>" <hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>>
Date: 2018/09/04 02:18
Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
________________________________
Hi Gustavo and Michihiro,
we noticed jtreg test failures when using this change:
compiler/runtime/safepoints/TestRegisterRestoring.java
compiler/runtime/Test7196199.java
TestRegisterRestoring is a simple test which returns arbitrary results instead of 10000.
We didn't see it on all machines, so it might be an issue with saving&restoring VR registers in the signal handler.
The machine which I have used has "SUSE Linux Enterprise Server 12 SP3" with kernel 4.4.126-94.22-default.
That's what I found out so far. Maybe you have an idea?
I also noticed that "-XX:-SuperwordUseVSX" crashes with bad ad file when your patch is applied. Looks like matching the vector nodes needs to be prevented.
Best regards,
Martin
-----Original Message-----
From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net<mailto:hotspot-dev-bounces at openjdk.java.net>> On Behalf Of Gustavo Romero
Sent: Montag, 3. September 2018 14:57
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>>; Michihiro Horie <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>; hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>
Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
Hi Goetz,
On 09/03/2018 09:27 AM, Lindenmaier, Goetz wrote:
> Also, I can not find all of the mail traffic in
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-August/thread.html.
> Is this a problem of the pipermail server?
>
> For some reason this webrev lacks the links to browse the diffs.
> Do you need to use a more recent webrev? You can obtain it with
> hg clone http://hg.openjdk.java.net/code-tools/webrev/ .
Yes, probably it was a problem of the pipermail or in some relay.
I noted the same thing, i.e. at least one Michi reply arrived
to me but missed a ML.
The initial discussion is here:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-July/003613.html
I understand Martin reviewed the last webrev in that thread, which is
http://cr.openjdk.java.net/~mhorie/8208171/webrev.01/ (taken from
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-July/003615.html)
Martin's review of webrev.01:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-August/033958.html
and Michi's reply to Martin's review of webrev.01:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-August/003632.html (with webrev.02,
taken from http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-August/003632.html).
and your last review:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-September/030419.html
HTH.
Best regards,
Gustavo
> Why do you rename vnoreg to vnoregi?
>
> Besides that the change is fine, thanks for implementing this!
>
> Best regards,
> Goetz.
>
>
>> -----Original Message-----
>> From: Doerr, Martin
>> Sent: Dienstag, 28. August 2018 19:35
>> To: Gustavo Romero <gromero at linux.vnet.ibm.com<mailto:gromero at linux.vnet.ibm.com>>; Michihiro Horie
>> <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>>; hotspot-
>> dev at openjdk.java.net<mailto:dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>; Simonis, Volker
>> <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>
>> 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<mailto:gromero at linux.vnet.ibm.com>>
>> Sent: Donnerstag, 26. Juli 2018 16:02
>> To: Michihiro Horie <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>>; hotspot-
>> dev at openjdk.java.net<mailto:dev at openjdk.java.net>; Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>; ppc-aix-
>> port-dev at openjdk.java.net<mailto:port-dev at openjdk.java.net>; Simonis, Volker <volker.simonis at sap.com<mailto: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<mailto:gromero at linux.vnet.ibm.com>>
>>> To: Michihiro Horie/Japan/IBM at IBMJP, ppc-aix-port-
>> dev at openjdk.java.net<mailto:dev at openjdk.java.net>, hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>
>>> Cc: goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>, volker.simonis at sap.com<mailto:volker.simonis at sap.com>, "Doerr, Martin"
>> <martin.doerr at sap.com<mailto: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/hotspot-compiler-dev/attachments/20180904/c074946b/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 105 bytes
Desc: image001.gif
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180904/c074946b/image001-0001.gif>
More information about the hotspot-compiler-dev
mailing list