[14] RFR (L): 8235405: C2: Merge AD instructions for different vector operations

[Viswanathan, Sandhya]

Hi Vladimir,

Thanks a lot for your review. Please see my response in your email below.

Best Regards,
Sandhya
 

-----Original Message-----
From: Vladimir Kozlov <vladimir.kozlov at oracle.com> 
Sent: Monday, December 09, 2019 4:44 PM
To: hotspot-compiler-dev at openjdk.java.net
Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
Subject: Re: [14] RFR (L): 8235405: C2: Merge AD instructions for different vector operations

On 12/5/19 4:01 AM, Vladimir Ivanov wrote:
> http://cr.openjdk.java.net/~vlivanov/jbhateja/8235405/webrev.00/all
> https://bugs.openjdk.java.net/browse/JDK-8235405
> 
> Reduce the number of AD instructions needed to implement vector 
> operations by merging existing ones. The patch covers the following operations:
>    - LoadVector
>    - StoreVector

There was difference in usage of vmovdquq vs vmovdqul instructions for 64 bytes wide vectors depending on element size (or number of elements in other word). The only difference in encoding of 2 instructions is value of *vex_w* attribute.
Proposed change use only vmovdqul. I want to know rational behind the change.

Sandhya >> The AVX512 emovdqul and emovdquq instruction behavior is same, when the instruction is encoded with no mask register, which is the case here. 

>    - RoundDoubleModeV

Conversion of predicate to assert vround8D_* instructions needs to be explained.
Originally they were guarded by UseAVX > 2. The code in match_rule_supported_vector() checks only AVX and not AVX512:

http://hg.openjdk.java.net/jdk/jdk/file/2aaa8bcb90a9/src/hotspot/cpu/x86/x86.ad#l1411

Is it because 8234392 added vector_size_supported() check?

Sandhya >> Yes.

>    - AndV
>    - OrV
>    - XorV

Above 3 are good.

>    - MulAddVS2VI

Both related webrevs are good.

>    - PopCountVI

Good.

Thanks,
Vladimir

> 
> Indiviual patches:
> 
> http://cr.openjdk.java.net/~vlivanov/jbhateja/8235405/webrev.00/indivi
> dual
> 
> As Jatin described, merging is applied only to AD instructions of 
> similar shape. There are some more opportunities for reduction/merging left, but they are deliberately left out for future work.
> 
> The patch is derived from the inintial version of generic vector 
> support [1]. Generic vector support was reviewed earlier and the other parts of refactorings in x86.ad  will be posted for review separately (7 more patches pending).
> 
> Testing: tier1-4, test run on different CPU flavors (KNL, SKL, etc)
> 
> Contributed-by: Jatin Bhateja <jatin.bhateja at intel.com>
> Reviewed-by: vlivanov, sviswanathan, ?
> 
> Best regards,
> Vladimir Ivanov
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-Augu
> st/034822.html