RFR 8190800: Support for vectorization of sqrt float

[Vladimir Kozlov]

Hi Razvan,

Very nice work. Thanks!

What testing you ran?

On 11/17/17 4:34 PM, Lupusoru, Razvan A wrote:
> Hi Jacek,
> 
> You are very welcome and it is nice we had same idea for implementation :)
> 
> Regarding your questions:
> 
> 1)I did not want to change other architecture .ad files because I do not 
> have the means to test them. Because of that, you will see that the 
> Ideal conversion restricts conversion to SqrtF node only for 
> architectures that have a Matcher entry. Namely all non-x86 
> architectures will work exact same way as before - this ensures I did 
> not break anything for them.

Yes, that is right approach.

> 
> 2)It appears that it is general convention in the .ad file that all 
> “packed” combinations are supported. Thus, you will find many places 
> where 2F is a supported combination. That is because the vectorizer may 
> decide (unlikely) that it should pack only 2 floats. The AVX instruction 
> will apply transform on all 4 lanes (when 128 bit), but the caller of 
> that .ad entry will only look at the resulting 2 relevant float lanes 
> (as reflected by dst being vecD). I have ensured now that all 2F entries 
> use vecD.

We have MaxVectorSize flag for testing purpose which limits vector size 
for debugging and purposes.
Also a loop can be unrolled only few times due to its code size and as 
result you can have less elements per vector than vector instructions 
support.
That is why we have all these packing combinations.

Razvan, please, run your testing with different MaxVectorSize (it is in 
bytes).

> 
> 3)You are right - that is a copy/paste error. It should say “float” 
> instead of “double”.
> 
> I have uploaded a fixed webrev here: 
> http://cr.openjdk.java.net/~rlupusoru/jdk_hs/webrev_sqrtf_03/

Looks good to me.

Thanks,
Vladimir

> 
> Thanks,
> 
> Razvan
> 
> *From:*Jacek Tomaka [mailto:jacekt at dug.com]
> *Sent:* Friday, November 17, 2017 4:03 PM
> *To:* Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>
> *Cc:* hotspot-compiler-dev at openjdk.java.net
> *Subject:* Re: RFR 8190800: Support for vectorization of sqrt float
> 
> Hi Razvan,
> 
> Thank you for making this patch available.
> 
> I have a few questions regarding your changes:
> 
> 1. Would it make sense to get rid of match(Set dst (ConvD2F (SqrtD 
> (ConvF2D src)))); for other platforms as well(and just use match(Set dst 
> (SqrtF src));), now that ConvD2FNode::Ideal does similar job?
> 
> 2. What would be situations where instruct vsqrt2F_reg and instruct 
> vsqrt2F_mem would be used? Isn't AVX only >128 bit?
> 
> 3. In subnode.hpp i think the comment should mention float not double. 
> It is +// square root a double currently.
> 
> Regards.
> 
> Jacek Tomaka
>