RFR: 8256973: Intrinsic creation for VectorMask query (lastTrue, firstTrue, trueCount) APIs [v2]

[Jatin Bhateja]

On Fri, 14 May 2021 12:48:58 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8256973: Review comments resolution.
>
> src/hotspot/cpu/x86/x86.ad line 8085:
> 
>> 8083: 
>> 8084: instruct vmask_true_count_avx(rRegI dst, vec mask, rRegL tmp, vec xtmp, vec xtmp1) %{
>> 8085:   predicate(!VM_Version::supports_avx512bw());
> 
> `VM_Version::supports_avx512vlbw()`

Handled in match_rule_supported_vector.

> src/hotspot/share/opto/vectornode.cpp line 1297:
> 
>> 1295: }
>> 1296: 
>> 1297: Node* VectorMaskOpNode::Ideal(PhaseGVN* phase, bool can_reshape) {
> 
> It doesn't make much sense to me. Why don't you simply require the input to be in canonical shape from the very beginning by unconditionally wrapping it into `VectorStoreMask` during construction?

Done

> src/hotspot/share/opto/vectornode.hpp line 858:
> 
>> 856: class VectorMaskOpNode : public TypeNode {
>> 857:  public:
>> 858:   VectorMaskOpNode(Node* mask, const Type* ty, const Type* ety, int mopc):
> 
> `ty`/`ety` caught my eye. It doesn't match anything in vectornode.hpp and may confuse readers.
> Any reason not to use `vt`? 
> 
> Also, any particular reason to cache full-blown type instead of capturing just the `BasicType`?

In this case all the mask operations produce an integer value.  Thus did not use vt, have removed ety since there its does have any direct use currently.

> src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 469:
> 
>> 467:     public static
>> 468:     <M>
>> 469:     int maskOp(int oper, Class<?> maskClass, Class<?> elemClass, int length, M m,
> 
> I second Paul here: `maskOp` case is already covered by `reductionCoerced`.

As discussed above mixing it with reduction coerced will require changes in original entry point (type parameter have Vector as the lower bound) , also we may need to bypass some irrelevant portions in inline_vector_reduction() , for the time being to keep the things clean added a different entry point for all masked operations.

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

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