On Fri, 6 Jun 2025 10:38:11 GMT, erifan <d...@openjdk.org> wrote:
>> This patch optimizes the following patterns:
>> For integer types:
>>
>> (XorV (VectorMaskCmp src1 src2 cond) (Replicate -1))
>> => (VectorMaskCmp src1 src2 ncond)
>> (XorVMask (VectorMaskCmp src1 src2 cond) (MaskAll m1))
>> => (VectorMaskCmp src1 src2 ncond)
>>
>> cond can be eq, ne, le, ge, lt, gt, ule, uge, ult and ugt, ncond is the
>> negative comparison of cond.
>>
>> For float and double types:
>>
>> (XorV (VectorMaskCast (VectorMaskCmp src1 src2 cond)) (Replicate -1))
>> => (VectorMaskCast (VectorMaskCmp src1 src2 ncond))
>> (XorVMask (VectorMaskCast (VectorMaskCmp src1 src2 cond)) (MaskAll m1))
>> => (VectorMaskCast (VectorMaskCmp src1 src2 ncond))
>>
>> cond can be eq or ne.
>>
>> Benchmarks on Nvidia Grace machine with 128-bit SVE2: With option
>> `-XX:UseSVE=2`:
>>
>> Benchmark Unit Before Score Error After
>> Score Error Uplift
>> testCompareEQMaskNotByte ops/s 7912127.225 2677.289518
>> 10266136.26 8955.008548 1.29
>> testCompareEQMaskNotDouble ops/s 884737.6799 446.963779
>> 1179760.772 448.031844 1.33
>> testCompareEQMaskNotFloat ops/s 1765045.787 682.332214
>> 2359520.803 896.305743 1.33
>> testCompareEQMaskNotInt ops/s 1787221.411 977.743935
>> 2353952.519 960.069976 1.31
>> testCompareEQMaskNotLong ops/s 895297.1974 673.44808
>> 1178449.02 323.804205 1.31
>> testCompareEQMaskNotShort ops/s 3339987.002 3415.2226
>> 4712761.965 2110.862053 1.41
>> testCompareGEMaskNotByte ops/s 7907615.16 4094.243652
>> 10251646.9 9486.699831 1.29
>> testCompareGEMaskNotInt ops/s 1683738.958 4233.813092
>> 2352855.205 1251.952546 1.39
>> testCompareGEMaskNotLong ops/s 854496.1561 8594.598885
>> 1177811.493 521.1229 1.37
>> testCompareGEMaskNotShort ops/s 3341860.309 1578.975338
>> 4714008.434 1681.10365 1.41
>> testCompareGTMaskNotByte ops/s 7910823.674 2993.367032
>> 10245063.58 9774.75138 1.29
>> testCompareGTMaskNotInt ops/s 1673393.928 3153.099431
>> 2353654.521 1190.848583 1.4
>> testCompareGTMaskNotLong ops/s 849405.9159 2432.858159
>> 1177952.041 359.96413 1.38
>> testCompareGTMaskNotShort ops/s 3339509.141 3339.976585
>> 4711442.496 2673.364893 1.41
>> testCompareLEMaskNotByte ops/s 7911340.004 3114.69191
>> 10231626.5 27134.20035 1.29
>> testCompareLEMaskNotInt ops/s 1675812.113 1340.969885
>> 2353255.341 1452.4522 1.4
>> testCompareLEMaskNotLong ops/s 848862.8036 6564.841731
>> 1177763.623 539.290106 1.38
>> testCompareLEMaskNotShort ops/s 3324951.54 2380.29473
>> 4712116.251 1544.559684 1.41
>> testCompareLTMaskNotByte ops/s 7910390.844 2630.861436
>> 10239567.69 6487.441672 1.29
>> testCompareLTMaskNotInt ops/s 16721...
>
> erifan has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Support negating unsigned comparison for BoolTest::mask
>
> Added a static method `negate_mask(mask btm)` into BoolTest class to
> negate both signed and unsigned comparison.
src/hotspot/share/opto/vectornode.cpp line 2221:
> 2219: // XorV/XorVMask is commutative, swap VectorMaskCmp/VectorMaskCast to
> in1.
> 2220: if (in2->Opcode() == Op_VectorMaskCmp ||
> 2221: (in2->Opcode() == Op_VectorMaskCast && in2->in(1)->Opcode() ==
> Op_VectorMaskCmp)) {
We may need to consider cases that a `VectorMaskCast` is generated between
`compare + not`, such as `compare + cast + not`. For such cases, the element
size maybe different for input and output of a `cast`. Although this patch's
intention is not for the latter pattern, current change have also covered it
well. Could you please add more test/jmh for all kinds of `cast` pattern here?
And I think the scope of this PR could be also extended to `compare + cast +
not`. WDYT?
src/hotspot/share/opto/vectornode.cpp line 2237:
> 2235: !VectorNode::is_all_ones_vector(in2)) {
> 2236: return nullptr;
> 2237: }
This part can be refined more clearly:
// Swap and put all_ones_vector to right
if (!VectorNode::is_all_ones_vector(in1)) {
swap(in1, in2);
}
// uncast mask
bool need_cast = false;
if (in1->Opcode() == Op_VectorMaskCast &&
in1->outcnt() == 1) {
assert(in1->bottom_type()->eq(bottom_type()), "");
need_cast = true;
in1 = in1->in(1);
}
// Check mask cmp pattern
if (in1->Opcode() != Op_VectorMaskCmp ||
in1->outcnt() > 1 ||
!in1->as_VectorMaskCmp()->predicate_can_be_negated()) {
return nullptr;
}
// Convert VectorMaskCmp + not
// Cast back
if (need_cast) {
res = new VectorMaskCastNode(phase->transform(res), vect_type());
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2139568985
PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2139578186
