On Wed, 28 May 2025 12:26:31 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> erifan has updated the pull request with a new target base due to a merge or >> a rebase. The incremental webrev excludes the unrelated changes brought in >> by the merge/rebase. The pull request contains 10 additional commits since >> the last revision: >> >> - Refactor the JTReg tests for compare.xor(maskAll) >> >> Also made a bit change to support pattern `VectorMask.fromLong()`. >> - Merge branch 'master' into JDK-8354242 >> - Refactor code >> >> Add a new function XorVNode::Ideal_XorV_VectorMaskCmp to do this >> optimization, making the code more modular. >> - Merge branch 'master' into JDK-8354242 >> - Update the jtreg test >> - Merge branch 'master' into JDK-8354242 >> - Addressed some review comments >> >> 1. Call VectorNode::Ideal() only once in XorVNode::Ideal. >> 2. Improve code comments. >> - Merge branch 'master' into JDK-8354242 >> - Merge branch 'master' into JDK-8354242 >> - 8354242: VectorAPI: combine vector not operation with compare >> >> 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 4... > > test/micro/org/openjdk/bench/jdk/incubator/vector/MaskCompareNotBenchmark.java > line 49: > >> 47: private static final VectorSpecies<Long> L_SPECIES = >> LongVector.SPECIES_MAX; >> 48: private static final VectorSpecies<Float> F_SPECIES = >> FloatVector.SPECIES_MAX; >> 49: private static final VectorSpecies<Double> D_SPECIES = >> DoubleVector.SPECIES_MAX; > > Are you taking `SPECIES_MAX` on purpose here, or could we take > `SPECIES_PREFERRED` instead? > @jatin-bhateja What is the best to do in these tests? I suppose best would be > to test with all vector lengths... Thanks for pointing out this @eme64 ! Per my understanding, `SPECIES_MAX` is almost the same with `SPECIES_PREFERRED` in this case which are all specified to the max vector size of a hardware. Since the max vector size is different on different architectures, not all vector lengths are supported to be intrinsified on a specified architecture like AArch64, especially the SVE arch with different vector register size. Hence, just testing the max species makes sense to me as this is a mid-end common transformation. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2113020416
