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

[erifan]

On Wed, 28 May 2025 12:16:23 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/vectornode.cpp line 2244:
>> 
>>> 2242:   // BoolTest doesn't support unsigned comparisons.
>>> 2243:   BoolTest::mask neg_cond =
>>> 2244:       (BoolTest::mask) (((VectorMaskCmpNode*) in1)->get_predicate() ^ 4);
>> 
>> What is the hard-coded `^ 4` here? This whole line looks like we are looking at internals of the `VectorMaskCmpNode` or its predicate, and we should probably do that in some method there? Or maybe it should be part of the `BoolTest(::mask)` interface?
>
> Also: You now cast `(VectorMaskCmpNode*) in1` twice. Can we not do `as_VectorMaskCmp()`? Or could we at least cast it only once, and then use it as `in1_mask_cmp` instead?

> What is the hard-coded ^ 4 here?

This is to negate the comparison condition. We can't use `BoolTest::negate()` here because the comparison condition may be **unsigned** comparison. Since there's already a `negate()` function in `BoolTest`, so I tend to add a new function `get_negative_predicate` for this into class `VectorMaskCmpNode`.

> Also: You now cast (VectorMaskCmpNode*) in1 twice. Can we not do as_VectorMaskCmp()? Or could we at least cast it only once, and then use it as in1_mask_cmp instead?

For the first cast, I think you mean

  if (in1->Opcode() != Op_VectorMaskCmp ||
      in1->outcnt() > 1 ||
      !((VectorMaskCmpNode*) in1)->predicate_can_be_negated() ||
      !VectorNode::is_all_ones_vector(in2)) {
    return nullptr;
  }

To remove one cast, then we have to split the above `if` because `in1` may not be a `VectorMaskCmpNode`.

  if (in1->Opcode() != Op_VectorMaskCmp) {
    return nullptr;
  }
  VectorMaskCmpNode* in1_as_mask_cmp = (VectorMaskCmpNode*) in1;
  if (in1->outcnt() > 1 ||
      !in1_as_mask_cmp->predicate_can_be_negated() ||
      !VectorNode::is_all_ones_vector(in2)) {
    return nullptr;
  }
 BoolTest::mask neg_cond = (BoolTest::mask) (in1_as_mask_cmp->get_predicate() ^ 4);

Does this look better to you ?

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

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