On Wed, 25 Feb 2026 13:11:28 GMT, Emanuel Peter <[email protected]> wrote:
>> Eric Fang has updated the pull request with a new target base due to a merge >> or a rebase. The pull request now contains 16 commits: >> >> - Improve the code comment and tests >> - Merge branch 'master' into JDK-8370863-mask-cast-opt >> - Refine the JTReg tests >> - Add clearer comments to VectorMaskCastIdentityTest.java >> - Update copyright year to 2026 >> - Merge branch 'master' into JDK-8370863-mask-cast-opt >> - Convert the check condition for vector length into an assertion >> >> Also refined the tests. >> - Refine code comments >> - Merge branch 'master' into JDK-8370863-mask-cast-opt >> - Merge branch 'master' into JDK-8370863-mask-cast-opt >> - ... and 6 more: https://git.openjdk.org/jdk/compare/c0c1775a...dcd64ad1 > > I'm getting some failures with your new test: > > > Failed IR Rules (1) of Methods (1) > ---------------------------------- > 1) Method > "compiler.vectorapi.VectorStoreMaskIdentityTest::testVectorMaskStoreIdentityFloat256" > - [Failed IR rules: 1]: > * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, > applyIfPlatformAnd={}, applyIfCPUFeatureOr={"sve", "true", "avx2", "true"}, > counts={"_#V#LOAD_VECTOR_Z#_", "_@4", ">= 3", "_#VECTOR_LOAD_MASK#_", "= 0", > "_#VECTOR_STORE_MASK#_", "= 0", "_#VECTOR_MASK_CAST#_", "= 0"}, failOn={}, > applyIfPlatform={}, applyIfPlatformOr={}, applyIfOr={}, > applyIfCPUFeatureAnd={}, applyIf={"MaxVectorSize", "> 16"}, > applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})" > > Phase "PrintIdeal": > - counts: Graph contains wrong number of nodes: > * Constraint 2: "(\\d+(\\s){2}(VectorLoadMask.*)+(\\s){2}===.*)" > - Failed comparison: [found] 1 = 0 [given] > - Matched node: > * 277 VectorLoadMask === _ 106 [[ 275 ]] #vectormask<F,4> > !jvms: VectorMask::fromArray @ bci:47 (line 209) > VectorStoreMaskIdentityTest::testTwoCastsKernel @ bci:5 (line 75) > VectorStoreMaskIdentityTest::testVectorMaskStoreIdentityFloat256 @ bci:29 > (line 271) > * Constraint 3: "(\\d+(\\s){2}(VectorStoreMask.*)+(\\s){2}===.*)" > - Failed comparison: [found] 1 = 0 [given] > - Matched node: > * 254 VectorStoreMask === _ 271 272 [[ 216 ]] > #vectors<Z,4> !jvms: AbstractMask::intoArray @ bci:50 (line 75) > VectorStoreMaskIdentityTest::testTwoCastsKernel @ bci:20 (line 77) > VectorStoreMaskIdentityTest::testVectorMaskStoreIdentityFloat256 @ bci:29 > (line 271) > * Constraint 4: "(\\d+(\\s){2}(VectorMaskCast.*)+(\\s){2}===.*)" > - Failed comparison: [found] 2 = 0 [given] > - Matched nodes (2): > * 271 VectorMaskCast === _ 275 [[ 254 ]] #vectormask<J,4> > !jvms: ShortVector64$ShortMask64::cast @ bci:54 (line 665) > VectorStoreMaskIdentityTest::testTwoCastsKernel @ bci:13 (line 77) > VectorStoreMaskIdentityTest::testVectorMaskStoreIdentityFloat256 @ bci:29 > (line 271) > * 275 VectorMaskCast === _ 277 [[ 271 ]] #vectormask<S,4> > !orig=[5222],[2038] !jvms: FloatVector128$FloatMask128::cast @ bci:54 (line > 654) VectorStoreMaskIdentityTest::testTwoCastsKernel @ bci:9 (line 76) > VectorStoreMaskIdentityTest::testVectorMaskStoreIdentityFloat256 @ bci:29 > (line 271) > > > This was run on an x64 machine with extra flags: > `-ea -esa -XX:CompileThreshold=100 ... Hi @eme64 , I looked at this test failure. I couldn't reproduce it with a small java case, but I suspect the problem is related to inlining and the IGVN processing order. The relevant optimization is `VectorStoreMask (VectorMaskCast* VectorLoadMask bv) elem_size ==> bv` (in [VectorStoreMaskNode::Identity()](https://github.com/openjdk/jdk/pull/28313/changes#diff-692826251cae892bc4737919579c6afbd317551cd507f99c7bd29d585c1282e2R1515)). **This optimization doesn't seem to be triggered when the test fails**. I suspect the reason might be this: Because the test uses three @ForceInline helper functions `testOneCastKernel`, `testTwoCastKernel` and `testThreeCastKernel`, which might affect the inline order of the test functions. If the `VectorStoreMaskNode` is generated before `VectorMaskCastNode` or `VectorLoadMaskNode`, then the IR pattern won't match when running the `VectorStoreMaskNode::Identity()` function. If no further optimizations trigger `VectorStoreMaskNode::Identity()` to run again, then this optimization will ultimately not be triggered, so the relevant IR nodes are retained. I tried the following: 1. Removed all IR checks and ran the test 100 times, without failure. This indicates that the test failure is only due to IR matching failure, and there is no correctness issue. This can also be confirmed by the IR graph dumped after the failure. 2. Delay the optimization `VectorStoreMaskNode::Identity()` until after loop optimization, then run the test 100 times; all tests passed. git diff diff --git a/src/hotspot/share/opto/vectornode.cpp b/src/hotspot/share/opto/vectornode.cpp index e5c8e87bc25..d9ffa4835af 100644 --- a/src/hotspot/share/opto/vectornode.cpp +++ b/src/hotspot/share/opto/vectornode.cpp @@ -1508,6 +1508,10 @@ Node* VectorLoadMaskNode::Identity(PhaseGVN* phase) { } Node* VectorStoreMaskNode::Identity(PhaseGVN* phase) { + if (!phase->C->post_loop_opts_phase()) { + phase->C->record_for_post_loop_opts_igvn(this); + return this; + } // Identity transformation on boolean vectors. // VectorStoreMask (VectorMaskCast* VectorLoadMask bv) elem_size ==> bv // vector[n]{bool} => vector[n]{t} => vector[n]{bool} 3. Remove the three @ForceInline helper functions, run the test 100 times, and all passed. If my guess is correct, I believe this issue is unrelated to this PR. Therefore, my current approach is to remove the three @ForceInline helper functions. This makes all tests pass. Do you think this is okay? If you think it's okay, could you please run the internal tests again to confirm the latest changes? Thank you! ------------- PR Comment: https://git.openjdk.org/jdk/pull/28313#issuecomment-3982779763
