RFR: 8354242: VectorAPI: combine vector not operation with compare [v9]

[erifan]

On Mon, 7 Jul 2025 09:32:42 GMT, erifan <duke at openjdk.org> wrote:

>> src/hotspot/share/opto/vectornode.cpp line 2243:
>> 
>>> 2241:       !VectorNode::is_all_ones_vector(in2)) {
>>> 2242:     return nullptr;
>>> 2243:   }
>> 
>> Suggestion:
>> 
>>   if (in1->Opcode() != Op_VectorMaskCmp ||
>>       in1->outcnt() != 1 ||
>>       !(in1->as_VectorMaskCmp())->predicate_can_be_negated() ||
>>         !VectorNode::is_all_ones_vector(in2)) {
>>     return nullptr;
>>   }
>> 
>> Indentation for clarity.
>> 
>> Currently, you exiting if one of these is the case:
>> 1. Not `MaskCmp`
>> 2. More than one use
>> 3. predicate cannot be negated AND the vector is all ones. Can you explain this condition? Do you have tests for cases:
>>    - predicate negatable and vector not all ones
>>    - predircate not negatable and vector not all ones
>>    - predicate negatable and vector all ones
>>    - predicate not negatable and vectors all ones
>>  
>> Why do you guard against `VectorNode::is_all_ones_vector(in2)` at all?
>> 
>> The condition for 3. is easy to get wrong, so good testing is important here :)
>
> The current testing status for the conditions you listed:
>> 1. Not MaskCmp.
> 
> **No test for it, tested locally**, Because I think this condition is too straightforward.
> 
>> 2. More than one use.
> 
> **Tested**, see `VectorMaskCompareNotTest.java line 1118`.
> 
>> predicate negatable and vector not all ones. 
> 
> **Tested**, see `VectorMaskCompareNotTest.java line 1126`.
> 
>> predicate not negatable and vector not all ones. 
> 
> **No test for it**, because we have tests for `predicate not negatable` or `vector not all ones`. If either is `false`, return nullptr.
> 
>> predicate negatable and vector all ones. 
> 
> **A lot of tests for it**. For example `VectorMaskCompareNotTest.java line 1014`.
> 
>> predicate not negatable and vectors all ones. 
> 
> **Tested**, see `VectorMaskCompareNotTest.java line 1222`.

> Indentation for clarity.

Done.

I think we have enough negative tests. Please take a look at this PR, thanks~

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2191550171