On Fri, 20 Mar 2026 06:59:59 GMT, Emanuel Peter <[email protected]> wrote:
>> The prototype >> https://github.com/erifan/jdk/pull/new/JDK-8380424-miss-identity-opt >> >> Hi @eme64, the code for adding all nodes to the IGVN worklist is here. I >> personally think it's better to handle this issue separately, so I've >> separated it from this PR for now. If you think combining them would be more >> appropriate, I'd be happy to add the relevant changes to this PR. >> >> To be honest, the above changes don't fundamentally solve this >> Ideal/Identity missing problem; they only address it at this point (boxing >> elimination). Of course, given the special nature of the VectorAPI (which is >> greatly impacted by inlining and boxing), I think this approach is >> reasonable. >> >> And I'm also considering whether there is a more thorough solution, such as >> adding all nodes to the worklist and performing a full IGVN run in >> `PhaseRemoveUseLess`. Of course, the most important thing here is how to >> balance compilation time and performance. > > @erifan Yes, please do it separately, probably before this PR, right? > > Right, there are two parts to this - they can be separate tasks: > - After boxing elimination, we should make sure that all vector nodes (better > all nodes) are put on the worklist. This has some cost, but I think it is > acceptable. We could also make this conditional: only do it if you find any > vectors in the graph. > - We need to improve notification during IGVN, so that IGVN optimizations > that traverse more than a single hop always get triggered. We can verify that > with `VerifyIterativeGVN`. Hi @eme64 I created a PR for the missing identity optimizations, see https://github.com/openjdk/jdk/pull/30529. The PR #30529 is based on this PR #28313, because in this way I can write a stress test for the test failure we encountered in this PR. Of course, #30529 doesn't have to be based on #28313, so that the problem encountered in this PR wouldn't exist in #30529, and then we could enhance the IGVN notification for `VectorMaskCastNode` in this PR. It depends on which PR we want to be merged first. What do you think? ------------- PR Comment: https://git.openjdk.org/jdk/pull/28313#issuecomment-4168161100
