On Mon, 2 Mar 2026 10:04:57 GMT, Emanuel Peter <[email protected]> wrote:
>> Eric Fang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove the force inline helper functions in >> VectorStoreMaskIdentityTest.java > > Yes, I would vote for implementing the notification. > And if you want, you could then also work on implementing notification for > other Vector patterns, so we can remove the `VerifyIterativeGVN` exception. > That would make sure we really always apply these cool optimizations when > they could be applied ;) Hi @eme64 I tried to address this issue by tweaking the notification, but it turns out that doesn’t really help. The reason is that `add_users_of_use_to_worklist` only enqueues users into the IGVN worklist when a node actually changes during IGVN. For a particular inlining order, `VectorStoreMaskNode` never gets added to the IGVN worklist. And even if it does, the optimization still won’t fire if the full IR pattern is not yet in place. Because of that, I’m leaning toward a more general approach: after **incremental inlining** and **box/unbox elimination**, the vector nodes are essentially formed and the node graph is relatively clean. At that point, in `PhaseVector::do_cleanup()` we can add all vector nodes to the IGVN worklist, which helps trigger optimizations that might otherwise be missed. I ran into a very similar issue before when working on the[ “vector compare not” optimization](https://github.com/openjdk/jdk/pull/24674/changes#diff-a9c13c0c2dc96fe9239116ac75460fdffbbbebd6d44540ccf7ec5811c8fbf79eR1219), and related problems are commented in [PhaseIterGVN::verify_Identity_for](https://github.com/openjdk/jdk/blob/f26b379b97301aca49dda9cb5dfe9a5472af7140/src/hotspot/share/opto/phaseX.cpp#L2043) . I think this method applies well to this class of issues. I’ve implemented a prototype locally, and testing shows it fixes the failures seen in this PR. I also measured compilation times for various methods in [test/hotspot/jtreg/compiler/vectorapi/VectorStoreMaskIdentityTest.java](https://github.com/openjdk/jdk/pull/28313/changes#diff-293703ca13dd3f7932313e339faccbab007793d2a97fa4c29739291e134aa467R40) and they are essentially unchanged. Avg compile time of 12 runs: <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40";> <head> <meta name=ProgId content=Excel.Sheet> <meta name=Generator content="Microsoft Excel 15"> <link id=Main-File rel=Main-File href="file:////Users/erfang/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip.htm"> <link rel=File-List href="file:////Users/erfang/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip_filelist.xml"> </head> <body link="#467886" vlink="#96607D"> function | c2_compile_time_without_the change(ms) | c2_compile_time_with_the_change(ms) | after/before -- | -- | -- | -- testVectorMaskStoreIdentityByte | 24.92 | 24.89 | 1.00 testVectorMaskStoreIdentityShort | 22.17 | 22.06 | 0.99 testVectorMaskStoreIdentityInt | 19.42 | 19.39 | 1.00 testVectorMaskStoreIdentityLong | 17.00 | 17.00 | 1.00 testVectorMaskStoreIdentityFloat | 19.17 | 19.14 | 1.00 </body> </html> I also measured the time of `make images` with and without the change, almost no changes either: make images: before: real 2m20.504s user 52m50.980s sys 14m30.154s after: real 2m20.778s user 52m49.161s sys 14m34.085s Does this approach look acceptable to you? ------------- PR Comment: https://git.openjdk.org/jdk/pull/28313#issuecomment-3996562890
