[vectorIntrinsics+mask] RFR: 8264563: Add masked vector intrinsics for binary/store operations [v8]

[Vladimir Ivanov]

On Wed, 28 Apr 2021 06:47:55 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Hi, this is the basic masking support PR for Vector API mask operations on platforms like SVE/AVX-512. The main codes are from [1], which contains:
>> 
>> - The predicate register allocation for Arm SVE, and vector mask type definition.
>> - The basic optimization for parts of the mask operations with masking feature. It contains:   
>>  
>>   1. Vector API java implementation changes for masked binary/store.
>>   2. C2 compiler mid-end changes, including new vector intrinsics implementation and mask IRs.
>> 
>> Note that for easier discussion, this PR only provides the changes for limited masked operations (e.g. binary/store) and the mask generations (e.g. load/compare/maskAll). We will continue working on the following missing parts:
>> 
>> - Mask support for other operations (unary,ternary,reduction,load,etc.)
>> - More mask IRs implementation (and/or/xor, toVector, allTrue, anyTrue, trueCount, eq, etc)
>> - Vector boxing/unboxing support for mask type (deoptimization support for predicate registers)
>> 
>> Also note that this PR doesn't contain any backend implementations. So the blend pattern will be generated as before. Regarding to the AArch64 SVE backend support, we will create a separate PR based on this one in future.
>>   
>> [1] https://github.com/openjdk/panama-vector/pull/40
>> 
>> See more details from:
>> http://cr.openjdk.java.net/~xgong/rfr/mask/Vector%20API%20masking%20support%20proposal%20for%20Arm%20SVE.pdf
>> http://cr.openjdk.java.net/~xgong/rfr/mask/VectorAPI%20masking%20support.pdf
>> 
>> Any suggestions and discussions are welcome! Thanks a lot!
>
> Xiaohong Gong has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Separate lanewiseTemplate for masked and non-masked operations

Overall, looks good. 

Just a couple of minor comments.

src/hotspot/share/opto/vectorIntrinsics.cpp line 453:

> 451:   bool is_masked_op = gvn().type(argument(n + 5)) != TypePtr::NULL_PTR;
> 452:   if (is_vector_mask(vbox_klass)) {
> 453:     assert(!is_masked_op, "mask operations do not need mask to control");

Another invariant to check here is mask input has to be either "always null" or "never null".

Currently, it will be checked when mask is unboxed, but it makes sense to make the check evident.

src/hotspot/share/opto/vectorIntrinsics.cpp line 585:

> 583:   if (is_masked_op && mask != NULL) {
> 584:     if (use_predicate) {
> 585:       // TODO: add predicate implementation for masked operation.

Does it make sense to fail the intrinsification or fail-fast with a crash here until the support is there?

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/DoubleVector.java line 811:

> 809:                                   long e, VectorMask<Double> m) {
> 810:         double e1 = (double) e;
> 811:         if ((long)e1 != e

Formatting is broken.

-------------

Marked as reviewed by vlivanov (Committer).

PR: https://git.openjdk.java.net/panama-vector/pull/57