RFR: 8333393: PhaseCFG::insert_anti_dependences can fail to raise LCAs and to add necessary anti-dependence edges [v13]

[Christian Hagedorn]

On Wed, 5 Feb 2025 17:37:34 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>> That sounds about right, yes! Thanks for persisting here. I'm really looking forward to what you find 😊
>
> Interestingly, this is the line in `cfgnode.cpp` that blocks the MergeMem/Phi swap idealization:
> 
>     // This restriction is temporarily necessary to ensure termination:
>     if (!saw_self && adr_type() == TypePtr::BOTTOM)  merge_width = 0;
> 
> If I comment out the line it solves all the failures we have seen. I double-checked that we then perform the exact MergeMem/Phi swap idealizations discussed above.
> 
> I am wondering what the proper solution is here. I will, of course, investigate if it is possible to loosen the restriction and still ensure termination. On the other hand, it also seems strange that the anti-dependence search is so sensitive to missing idealizations?

Thanks for having yet another look at this!

> If I comment out the line it solves all the failures we have seen. I double-checked that we then perform the exact 
MergeMem/Phi swap idealizations discussed above.

That sounds promising! Looks like this temporary restriction became quite permanent - it's from initial load. I'm wondering if that is still necessary and if so if we have tests to catch that (we would probably hit the "infinite loop in IGVN" in that case).

> I am wondering what the proper solution is here. I will, of course, investigate if it is possible to loosen the restriction and still ensure termination. On the other hand, it also seems strange that the anti-dependence search is so sensitive to missing idealizations?

That would be great if we can get around this termination issue somehow - if it's still a problem. I think that is very unfortunate that we might be relying on this Ideal transformation to be applied to ensure correctness later on. If it's really required, we should at least make sure to add some verification code to catch this in debug builds. You could, for example, just turn what you have now into verification code, i.e. check that we cannot find another anti dependency edge with another search root. And/Or re-apply this particular transformation for each Phi node again in the end to see if we missed some swaps.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22852#discussion_r1944246463