RFR: 8264104: Eliminate unnecessary vector mask conversion during VectorUnbox for floating point VectorMask [v3]

[Vladimir Ivanov]

On Fri, 9 Apr 2021 07:43:50 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> The Vector API defines different element types for floating point VectorMask. For example, the bitwise related APIs use "`long/int`", while data related APIs use "`double/float`". This makes some optimizations that based on the type checking cannot work well.
>> 
>> For example, the VectorBox/Unbox elimination like `"VectorUnbox (VectorBox v) ==> v"` requires the types of output and
>> input are equal. Normally this is necessary. However, due to the different element type for floating point VectorMask with the same species, the VectorBox/Unbox pattern is optimized to:
>>   VectorLoadMask (VectorStoreMask vmask)
>> Actually the types can be treated as the same one for such cases. And considering the vector mask representation is the same for
>> vectors with the same element size and vector length, it's safe to do the optimization:
>>   VectorLoadMask (VectorStoreMask vmask) ==> vmask
>> The same issue exists for GVN which is based on the type of a node. Making the mask node's` hash()/cmp()` methods depends on the element size instead of the element type can fix it.
>
> Xiaohong Gong has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert changes to VectorLoadMask and add a VectorMaskCast for the same element size mask casting

Overall, looks good.

src/hotspot/share/opto/vectornode.cpp line 1236:

> 1234:         if (is_vector_mask) {
> 1235:           if (in_vt->length_in_bytes() == out_vt->length_in_bytes() &&
> 1236:               Matcher::match_rule_supported(Op_VectorMaskCast)) {

There's `Matcher::match_rule_supported_vector()` specifically for vector nodes.

src/hotspot/share/opto/vectornode.cpp line 1237:

> 1235:           if (in_vt->length_in_bytes() == out_vt->length_in_bytes() &&
> 1236:               Matcher::match_rule_supported(Op_VectorMaskCast)) {
> 1237:             // VectorUnbox (VectorBox vmask) ==> VectorMaskCast (vmask)

It's better to implement it as a 2-step transformation and place it in `VectorLoadMaskNode::Ideal()`:
VectorUnbox (VectorBox vmask) ==> VectorLoadMask (VectorStoreMask vmask) => VectorMaskCast (vmask)

src/hotspot/share/opto/vectornode.hpp line 1247:

> 1245:     assert(type2aelembytes(in_vt->element_basic_type()) == type2aelembytes(vt->element_basic_type()), "element size must match");
> 1246:   }
> 1247: 

FTR there's a way to avoid platform-specific changes (in AD files) if we don't envision any non-trivial conversions to be supported: just disable matching of the node by overriding `Node::ideal_reg()`:
virtual uint ideal_reg() const { return 0; } // not matched in the AD file

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

Marked as reviewed by vlivanov (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3238