On Wed, 28 May 2025 12:16:23 GMT, Emanuel Peter <epe...@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
