RFR: 8292898: [vectorapi] Unify vector mask cast operation [v2]

[Jatin Bhateja]

On Wed, 7 Sep 2022 09:22:11 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> The current implementation of the vector mask cast operation is
>> complex that the compiler generates different patterns for different
>> scenarios. For architectures that do not support the predicate
>> feature, vector mask is represented the same as the normal vector.
>> So the vector mask cast is implemented by `VectorCast `node. But this
>> is not always needed. When two masks have the same element size (e.g.
>> int vs. float), their bits layout are the same. So casting between
>> them does not need to emit any instructions.
>> 
>> Currently the compiler generates different patterns based on the
>> vector type of the input/output and the platforms. Normally the
>> "`VectorMaskCast`" op is only used for cases that doesn't emit any
>> instructions, and "`VectorCast`" op is used to implement the necessary
>> expand/narrow operations. This can avoid adding some duplicate rules
>> in the backend. However, this also has the drawbacks:
>> 
>>  1) The codes are complex, especially when the compiler needs to
>>     check whether the hardware supports the necessary IRs for the
>>     vector mask cast. It needs to check different patterns for
>>     different cases.
>>  2) The vector mask cast operation could be implemented with cheaper
>>     instructions than the vector casting on some architectures.
>> 
>> Instead of generating `VectorCast `or `VectorMaskCast `nodes for different
>> cases of vector mask cast operations, this patch unifies the vector
>> mask cast implementation with "`VectorMaskCast`" node for all vector types
>> and platforms. The missing backend rules are also added for it.
>> 
>> This patch also simplies the vector mask conversion happened in
>> "`VectorUnbox::Ideal()`". Normally "`VectorUnbox (VectorBox vmask)`" can
>> be optimized to "`vmask`" if the unboxing type matches with the boxed
>> "`vmask`" type. Otherwise, it needs the type conversion. Currently the
>> "`VectorUnbox`" will be transformed to two different patterns to implement
>> the conversion:
>> 
>>  1) If the element size is not changed, it is transformed to:
>> 
>>     "VectorMaskCast vmask"
>> 
>>  2) Otherwise, it is transformed to:
>> 
>>     "VectorLoadMask (VectorStoreMask vmask)"
>> 
>> It firstly converts the "`vmask`" to a boolean vector with "`VectorStoreMask`",
>> and then uses "`VectorLoadMask`" to convert the boolean vector to the
>> dst mask vector. Since this patch makes "`VectorMaskCast`" op supported
>> for all types on all platforms, it doesn't need the "`VectorLoadMask`" and
>> "`VectorStoreMask`" to do the conversion. The existing transformation:
>> 
>>   VectorUnbox (VectorBox vmask) => VectorLoadMask (VectorStoreMask vmask)
>> 
>> can be simplified to:
>> 
>>   VectorUnbox (VectorBox vmask) => VectorMaskCast vmask
>
> Xiaohong Gong has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains one additional commit since the last revision:
> 
>   8292898: [vectorapi] Unify vector mask cast operation

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4630:

> 4628:         } else {
> 4629:           vpmovsxbw(dst, src, vlen_enc);
> 4630:         }

if int/short == 2  and target == AVX512BW:  VPMOVSXBW looks incorrect, expectation is to wide cast each 16 bit lane to 32 bit and not 8 bit to 16 bit.

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4633:

> 4631:         break;
> 4632:       }
> 4633:       case 4: vpmovsxbd(dst, src, vlen_enc); break;

long/short == 4:  8 bit to 16 bit lane casting looks incorrect, it should be 16 bits to 64 bits.

src/hotspot/cpu/x86/x86.ad line 8420:

> 8418: 
> 8419: instruct vmaskcast_avx(vec dst, vec src) %{
> 8420:   predicate(Matcher::vector_length(n) == Matcher::vector_length(n->in(1)) &&

Redundant predication for length equality check.

src/hotspot/cpu/x86/x86.ad line 8438:

> 8436: instruct vmaskcast_avx1_32B_expand(vec dst, vec src, vec xtmp1, vec xtmp2) %{
> 8437:   predicate(UseAVX == 1 && Matcher::vector_length_in_bytes(n) == 32 &&
> 8438:             Matcher::vector_length(n) == Matcher::vector_length(n->in(1)) &&

Redundant length check.

src/hotspot/cpu/x86/x86.ad line 8455:

> 8453: instruct vmaskcast_avx1_32B_shrink(vec dst, vec src, vec xtmp) %{
> 8454:   predicate(UseAVX == 1 && Matcher::vector_length_in_bytes(n->in(1)) == 32 &&
> 8455:             Matcher::vector_length(n) == Matcher::vector_length(n->in(1)) &&

Redundant length check.

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

PR: https://git.openjdk.org/jdk/pull/10192