On Wed, 11 Jun 2025 08:47:56 GMT, Xiaohong Gong wrote:
>> 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
On Wed, 11 Jun 2025 07:43:55 GMT, Emanuel Peter wrote:
>> `VectorOperators.XXX` is not compile time constants, we can't use `switch`
>> here.
>
> Oh. Ok. Well at least add a `RuntimeException` to an `else` branch then, I
> would suggest :)
Make sense!
-
PR Review Comment: https:/
On Wed, 11 Jun 2025 08:30:51 GMT, Emanuel Peter wrote:
>>> You are checking IRNode.XOR_VL, "= 0". But you are comparing floats. Does
>>> that make sense?
>>
>> The bottom types of `float` and `double` vector masks are casted to `int`
>> and `long`. Seems this is by design? So this is correct.
On Wed, 11 Jun 2025 05:31:00 GMT, Emanuel Peter wrote:
> You are checking IRNode.XOR_VL, "= 0". But you are comparing floats. Does
> that make sense?
The bottom types of `float` and `double` vector masks are casted to `int` and
`long`. Seems this is by design? So this is correct.
As for `cont
On Wed, 11 Jun 2025 05:23:12 GMT, Emanuel Peter wrote:
>> 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
On Fri, 6 Jun 2025 10:38:11 GMT, erifan wrote:
>> This patch optimizes the following patterns:
>> For integer types:
>>
>> (XorV (VectorMaskCmp src1 src2 cond) (Replicate -1))
>> => (VectorMaskCmp src1 src2 ncond)
>> (XorVMask (Ve
On Tue, 10 Jun 2025 02:38:29 GMT, Fei Yang wrote:
> FYI: I submitted to testing in QEMU-system / Ubuntu 25.04 (fastdebug jdk
> build and 256-bit RVV) and I see `compiler/vectorization`,
> `compiler/vectorapi` and `jdk_vector` tests are passing.
Thank you very much!
-
PR Comment:
On Fri, 6 Jun 2025 10:38:11 GMT, erifan wrote:
>> This patch optimizes the following patterns:
>> For integer types:
>>
>> (XorV (VectorMaskCmp src1 src2 cond) (Replicate -1))
>> => (VectorMaskCmp src1 src2 ncond)
>> (XorVMask (Ve
On Fri, 6 Jun 2025 07:01:58 GMT, erifan wrote:
> > > Oh I think we still cannot use `BoolTest::negate`, because we cannot
> > > instantiate a `BoolTest` object with **unsigned** comparison.
> > > `BoolTest::negate` is a non-static function.
> >
> >
>
t; 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
On Thu, 5 Jun 2025 11:05:48 GMT, Emanuel Peter wrote:
> > Oh I think we still cannot use `BoolTest::negate`, because we cannot
> > instantiate a `BoolTest` object with **unsigned** comparison.
> > `BoolTest::negate` is a non-static function.
>
> I see. Ok. Hmm. I still think that the logic sho
On Thu, 5 Jun 2025 09:24:10 GMT, Emanuel Peter wrote:
> FYI: `BoolTest::negate` already does what you want: `mask negate( ) const {
> return mask(_test^4); }` I think you should use that instead :)
Indeed, I hadn't noticed that, thank you.
-
PR Comment: https://git.openjdk.org/jdk
On Wed, 28 May 2025 12:18:15 GMT, Emanuel Peter 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 additi
On Thu, 29 May 2025 01:44:49 GMT, Xiaohong Gong wrote:
>> test/micro/org/openjdk/bench/jdk/incubator/vector/MaskCompareNotBenchmark.java
>> line 49:
>>
>>> 47: private static final VectorSpecies L_SPECIES =
>>> LongVector.SPECIES_MAX;
>>> 48: private static final VectorSpecies F_SPECIE
On Thu, 5 Jun 2025 09:32:15 GMT, erifan wrote:
> > FYI: `BoolTest::negate` already does what you want: `mask negate( ) const {
> > return mask(_test^4); }` I think you should use that instead :)
>
> Indeed, I hadn't noticed that, thank you.
Oh I think we still canno
On Wed, 28 May 2025 12:28:20 GMT, Emanuel Peter wrote:
>> test/hotspot/jtreg/compiler/vectorapi/VectorMaskCompareNotTest.java line 237:
>>
>>> 235: // Byte tests
>>> 236: @Test
>>> 237: @IR(counts = { IRNode.XOR_V_MASK, "= 0", IRNode.XOR_VB, "= 0" },
>>
>> Could you still assert the
On Thu, 29 May 2025 07:55:06 GMT, erifan 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?
t; 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
On Thu, 29 May 2025 08:00:05 GMT, erifan wrote:
>> src/hotspot/share/opto/vectornode.cpp line 2233:
>>
>>> 2231: if (in2->Opcode() == Op_VectorMaskCast) {
>>> 2232: in2 = in2->in(1);
>>> 2233: }
>>
>> Wow, this seems to be an add
On Wed, 28 May 2025 12:03:48 GMT, Emanuel Peter 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 additi
On Wed, 28 May 2025 12:12:50 GMT, Emanuel Peter 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 additi
On Fri, 16 May 2025 07:40:53 GMT, erifan 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 com
On Wed, 28 May 2025 12:16:23 GMT, Emanuel Peter 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
On Wed, 14 May 2025 02:44:14 GMT, erifan wrote:
>> This patch optimizes the following patterns:
>> For integer types:
>>
>> (XorV (VectorMaskCmp src1 src2 cond) (Replicate -1))
>> => (VectorMaskCmp src1 src2 ncond)
>> (XorVMask (Ve
On Wed, 7 May 2025 11:13:23 GMT, Jatin Bhateja 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 eight
On Fri, 9 May 2025 09:37:22 GMT, erifan wrote:
>> Yes, converting `VectorMask.fromLong(SPECIES, -1L)` to `MaskAll()` would be
>> better, and that will benefit AArch64 as well, since `MaskAll()` is much
>> more cheaper than `fromLong()` on AArch64. We can add such a trans
t; 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
On Thu, 8 May 2025 01:49:45 GMT, Xiaohong Gong wrote:
>> Yes, that's the right approach. For this PR, I think you can mix some test
>> points covering compare, xor(maskAll(true)).
>
> Yes, converting `VectorMask.fromLong(SPECIES, -1L)` to `MaskAll()` would be
> better, and that will benefit AAr
On Wed, 7 May 2025 02:10:56 GMT, erifan wrote:
>> This patch optimizes the following patterns:
>> For integer types:
>>
>> (XorV (VectorMaskCmp src1 src2 cond) (Replicate -1))
>> => (VectorMaskCmp src1 src2 ncond)
>> (XorVMask (Ve
On Wed, 7 May 2025 06:59:34 GMT, Jatin Bhateja 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 eight
On Wed, 7 May 2025 06:59:34 GMT, Jatin Bhateja 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 eight
On Fri, 2 May 2025 06:14:33 GMT, Emanuel Peter wrote:
>> src/hotspot/share/opto/vectornode.cpp line 2216:
>>
>>> 2214: in2->is_predicated_vector()) {
>>> 2215: with_predicated = true;
>>> 2216: }
>>
>> Suggestion:
>>
>> bool with_predicated = is_predicated_vector() ||
>>
On Fri, 2 May 2025 06:16:03 GMT, Emanuel Peter 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 six additi
On Thu, 1 May 2025 07:32:22 GMT, erifan wrote:
>> Yes, this discussion is down to `requires` vs `applyIf`. This is my argument
>> for `applyIf`, quoted from above, I have not yet seen an argument against it:
>>
>>> If you use @require, then the person does not
t; 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
On Tue, 29 Apr 2025 10:22:22 GMT, Emanuel Peter 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 four additi
t; 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
On Tue, 29 Apr 2025 10:22:22 GMT, Emanuel Peter wrote:
> Yes, this discussion is down to `requires` vs `applyIf`. This is my argument
> for `applyIf`, quoted from above, I have not yet seen an argument against it:
>
> > If you use @require, then the person does not realize there is a test AND
atch the bug.
> > > > >
> > > > >
> > > > > Just copy pasting the IR applyIf everywhere is not that much work,
> > > > > and adding in a new platform later is not really hard either.
> > > >
> > > >
> > >
On Mon, 28 Apr 2025 14:12:02 GMT, Emanuel Peter wrote:
> I suppose in that case you can assert that you NEVER get those nodes, because
> if you have vectors not supported, they will not show up because of that, and
> if you do support vectors, they should be optimized away.
This is expected.
On Mon, 28 Apr 2025 14:10:40 GMT, Emanuel Peter wrote:
>> test/hotspot/jtreg/compiler/vectorapi/VectorMaskCompareNotTest.java line 237:
>>
>>> 235: public static void testCompareEQMaskNotByte() {
>>> 236: testCompareMaskNotByte(VectorOperators.EQ);
>>> 237: }
>>
>> Another comme
ere is not that much work, and
> > > adding in a new platform later is not really hard either.
> >
> >
> > Thanks! The problem is that when a new platform is added, people may not
> > even know there is a test.
>
> @erifan That is true. But we have that
On Mon, 28 Apr 2025 06:45:58 GMT, Emanuel Peter wrote:
>> This is not specifically required on x86, but because this test fails on x86
>> when `-XX:UseAVX=0` is specified. When `-XX:UseAVX=0` is specified, the
>> sub-graph is like this:
>> `(XorV (VectorMaskCmp (LoadVector ...)) (Replicate -1))
On Mon, 28 Apr 2025 06:47:38 GMT, Emanuel Peter wrote:
> Just a drive-by comment for now, I may review this later more fully.
>
> > I would also prefer if you added the IR restrictions rather than the JTREG
> > requires.
> > The benefit is that we can still run the tests on all platforms, at le
On Fri, 25 Apr 2025 09:48:59 GMT, Jatin Bhateja wrote:
>> Since this is a platform independent optimization, I tend to use this
>> `@requires` because it's simpler. If we use `applyIfCPUFeatureOr`, we need
>> to add the same restriction before each test. In addition, if a new
>> architecture
On Thu, 24 Apr 2025 10:28:15 GMT, Andrew Haley 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 two additi
On Thu, 24 Apr 2025 09:39:58 GMT, erifan wrote:
>> src/hotspot/share/opto/vectornode.cpp line 2265:
>>
>>> 2263: vmcmp = new VectorMaskCastNode(phase->transform(vmcmp),
>>> vmcast_vt);
>>> 2264: }
>>> 2265: return vmcmp;
>>
&g
t; 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
On Wed, 23 Apr 2025 12:09:51 GMT, Jatin Bhateja 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 two additi
On Fri, 18 Apr 2025 01:36:10 GMT, erifan wrote:
>> This patch optimizes the following patterns:
>> For integer types:
>>
>> (XorV (VectorMaskCmp src1 src2 cond) (Replicate -1))
>> => (VectorMaskCmp src1 src2 ncond)
>> (XorVMask (Ve
t; 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
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, u
52 matches
Mail list logo
