On Thu, 29 May 2025 07:55:06 GMT, erifan <d...@openjdk.org> wrote:
>> 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 ?
For now I kept the current approach, as I feel it's a little more compact.
Thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2128358563
