RFR: 8349479: C2: when a Type node becomes dead, make CFG path that uses it unreachable [v2]

[Christian Hagedorn]

On Thu, 13 Mar 2025 17:02:00 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The goal of the patch is to free us from having to worry about Type nodes becoming dead on a cfg path that doesn't fold ever again.

That's surely a strong advantage. It would also solve the problem of `CastII` becoming top on the uncommon path of a div by zero check. AFAIR, we have not solved that problem but just worked around it by removing the `CastII::Value()` optimization to improve types on such paths - which was not satisfying but worked. That optimization could be reintroduced as well as a follow-up to this patch.

> With this patch, there's no need for assert predicates AFAIU.

I've thought about that, too. But I think they are not completely useless: IIUC, if we have a `CastII` that's only used on some branch inside the loop, we would now just make this one branch unreachable with this patch but not clean up entire loop (which is actually dead). Assertion Predicates on the other hand kill the entire loop and prevent us from spending more time trying to optimize it in any way.

Another consideration for Assertion Predicates are data updates when splitting loops further that we applied Loop Predication to: We need to ensure that we correctly update all data dependencies on hoisted checks to avoid executing them too early. These data dependencies are hard to keep track of when we want to update them and don't have dedicated nodes where they can always be found at. Template Assertion Predicates solve this because they are only removed once loop opts are over. But if we only need them for keeping track of data dependencies, they can of course become a lot easier and serve as nops.

So, I think Assertion Predicates can still be useful as part of Loop Predication and Range Check Elimination. But we have many more places where `Type` nodes could die and break the graph. This patch can indeed solve this in general and I'm definitely in favor of having this patch to solve this problem and future problems with `Type` nodes once and for all.

> That's actually the first thing I tried but that doesn't work. A Cast can sometimes be dependent on some condition that was hoisted. What makes the Cast top is both some condition the Cast is control dependent on and the type of its input which can itself be control dependent on some control flow that's dominated by the control of the Cast.

I see, makes sense.

> Cost in terms of compilation time could be evaluated the usual way. I haven't done any of that but I could.

If we agree that we do not see an alternative approach to solve this problems, I think we could accept some increased compilation time. Anyhow, would still be good to get some numbers to have a better picture.

>  have no solution other than fragile workarounds or new complicated construct that will leak in a lot of parts of the compiler 

I guess when moving forward with this patch we should still consider how easy it is to keep data and control in sync when adding new `Type` nodes and not just rely on this patch to make things right. We might otherwise miss to remove some other dead nodes.

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

PR Comment: https://git.openjdk.org/jdk/pull/23468#issuecomment-2724311497