RFR: 8317723: C2: CountedLoopEndNodes and Zero Trip Guards are wrongly treated as Runtime Predicate

Wed Nov 15 08:08:29 UTC 2023

On Tue, 14 Nov 2023 08:38:12 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> The solution looks reasonable.
> 
> But I wonder if there could not be a scenario, where we have some "normal" if (i.e. a condition stated by the user in java code), which has a loop on both sides of the condition. If now one side collapses to a bare uncommon trap, this if would look like a runtime predicate, right? Is this impossible? If not, we may need a more fundamental solution. Maybe some new (sub)class of node, or a flag in the If node.

I've initially thought about marking Runtime Predicates with a flag or something like that. However, when one gets replaced by another node, we need to check if the new node should also be marked as a Runtime Predicate. I'm not sure how easy this is to get it right in all the cases. But doing this marking is probably not worth doing, because I think that, in theory, it is okay to treat normal `If` and `RangeCheck` nodes as Runtime Predicate even though they initially started out as non-predicates and only became one later due to folding one path to an uncommon trap path. It would even fit the definition of a Runtime Predicate: Check something and if it fails trap.

With `CountedLoopEndNodes` we kinda have the free information that this node has not started out as a Runtime Predicate. So, stopping here always seems right.

There is also code that wants to find the zero trip guard check from a loop head. Wrongly treating a zero trip guard as Runtime Predicate and skipping over it could possibly become a problem (e.g. code that asserts that we must have a zero trip guard could fail etc.). However, I have not found a case where we currently crash (without the new code in this patch to stop at zero trip guards). 

I therefore think updating the code to exclude these two cases is fine and enough for our purposes. What do you think?

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

PR Comment: https://git.openjdk.org/jdk/pull/16596#issuecomment-1811978104