On Tue, 8 Jul 2025 11:42:18 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> Oh wow, my bad. I misunderstood the brackets!
>>
>> Instead of:
>>
>> !(in1->as_VectorMaskCmp())->predicate_can_be_negated() ||
>> !VectorNode::is_all_ones_vector(in2)) {
>>
>> I read:
>>
>> !(in1->as_VectorMaskCmp()->predicate_can_be_negated() ||
>> !VectorNode::is_all_ones_vector(in2))) {
>>
>> That confused me a lot... absolutely my bad.
>>
>> Well actually then my indentation suggestion was terrible!
>
> I made a new suggestion below.
> A code comment would be helpful for this case.
I updated the comment above the code a bit. As for why predicate need to be
negatable, it's straightforward, the key of this optimization is to change
predicate condition into negative predicate condition. And in
`predicate_can_be_negated`, there's a comment explaining when predicate can't
be negated.
> I made a new suggestion below.
Done.
> That confused me a lot... absolutely my bad.
Well actually then my indentation suggestion was terrible!
No problem. I'm a newbie in the JDK community, so generally I think your
suggestions are valuable.Thanks for your review!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2194130234
