<html><body><p><font size="2">Hi Gustavo,</font><br><br><font size="2">Thank you very much for your helpful comments!</font><br><br><font size="2">I updated webrev:</font><br><a href="http://cr.openjdk.java.net/~mhorie/8208171/webrev.01/"><font size="2">http://cr.openjdk.java.net/~mhorie/8208171/webrev.01/</font></a><br><br><br><font size="2">Best regards,</font><br><font size="2">--</font><br><font size="2">Michihiro,</font><br><font size="2">IBM Research - Tokyo</font><br><br><img width="16" height="16" src="cid:1__=8FBB0845DF8A45A98f9e8a93df938690918c8FB@" border="0" alt="Inactive hide details for Gustavo Romero ---2018/07/25 23:05:32---Hi Michi, On 07/25/2018 02:43 AM, Michihiro Horie wrote:"><font size="2" color="#424282">Gustavo Romero ---2018/07/25 23:05:32---Hi Michi, On 07/25/2018 02:43 AM, Michihiro Horie wrote:</font><br><br><font size="2" color="#5F5F5F">From:        </font><font size="2">Gustavo Romero <gromero@linux.vnet.ibm.com></font><br><font size="2" color="#5F5F5F">To:        </font><font size="2">Michihiro Horie/Japan/IBM@IBMJP, ppc-aix-port-dev@openjdk.java.net, hotspot-dev@openjdk.java.net</font><br><font size="2" color="#5F5F5F">Cc:        </font><font size="2">goetz.lindenmaier@sap.com, volker.simonis@sap.com, "Doerr, Martin" <martin.doerr@sap.com></font><br><font size="2" color="#5F5F5F">Date:        </font><font size="2">2018/07/25 23:05</font><br><font size="2" color="#5F5F5F">Subject:        </font><font size="2">Re: RFR: 8208171: PPC64: Enrich SLP support</font><br><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br><br><br><tt><font size="2">Hi Michi,<br><br>On 07/25/2018 02:43 AM, Michihiro Horie wrote:<br>> Dear all,<br>> <br>> Would you review the following change?<br>> Bug: </font></tt><tt><font size="2"><a href="https://bugs.openjdk.java.net/browse/JDK-8208171">https://bugs.openjdk.java.net/browse/JDK-8208171</a></font></tt><tt><font size="2"><br>> Webrev: </font></tt><tt><font size="2"><a href="http://cr.openjdk.java.net/~mhorie/8208171/webrev.00">http://cr.openjdk.java.net/~mhorie/8208171/webrev.00</a></font></tt><tt><font size="2"><br>> <br>> This change adds support for vectorized arithmetic calculation with SLP.<br>> <br>> 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.<br><br>Looks good. Just a few comments:<br><br>- In vmul4F_reg() would it be reasonable to use xvmulsp instead of vmaddfp in<br>   order to avoid the splat?<br><br>- Although all instructions added by your change where introduced in ISA 2.06,<br>   so POWER7 and above are OK, as I see probes for PowerArchictecturePPC64=6|5 in<br>   vm_version_ppc.cpp (line 64),  I'm wondering if there is any control point to<br>   guarantee that these instructions won't be emitted on a CPU that does not<br>   support them.<br><br>- I think that in general string in format %{} are in upper case. For instance,<br>   this the current output on optoassembly for vmul4F:<br><br>2941835 5b4     ADDI    R24, R24, #64<br>2941836 5b8     vmaddfp  VSR32,VSR32,VSR36      ! mul packed4F<br>2941837 5c0     STXVD2X     [R17], VSR32        // store 16-byte Vector<br><br>   I think it would be better to be in upper case instead. I also think that if<br>   the node match emits more than one instruction all instructions must be listed<br>   in format %{}, since it's meant for detailed debugging. Finally I think it<br>   would be better to replace \t! by \t// in that string (unless I'm missing any<br>   special meaning for that char). So for vmul4F it would be something like:<br><br>2941835 5b4     ADDI      R24, R24, #64<br>                 VSPLTISW  VSR34, 0                 // Splat 0 imm in VSR34<br>2941836 5b8     VMADDFP   VSR32,VSR32,VSR36,VSR34  // Mul packed4F<br>2941837 5c0     STXVD2X   [R17], VSR32             // store 16-byte Vector<br><br><br>But feel free to change anything just after you get additional reviews :)<br><br><br>> I confirmed this change with JTREG. In addition, I used attached micro benchmarks.<br>> /(See attached file: slp_microbench.zip)/<br><br>Thanks for sharing it.<br>Btw, another option to host it would be in the CR<br>server, in </font></tt><tt><font size="2"><a href="http://cr.openjdk.java.net/~mhorie/8208171">http://cr.openjdk.java.net/~mhorie/8208171</a></font></tt><tt><font size="2"><br><br><br>Best regards,<br>Gustavo<br>  <br>> <br>> Best regards,<br>> --<br>> Michihiro,<br>> IBM Research - Tokyo<br>> <br></font></tt><br><br><BR>
</body></html>