RFR: 8335220: C2: Missing check for Opaque4 node in EscapeAnalysis

[Christian Hagedorn]

On Thu, 27 Jun 2024 05:33:11 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> While testing Leyden Early Release the problem was found in C2 EscapeAnalysis, in new code which handles allocations merge. `ConnectionGraph:: can_reduce_check_users()` have incomplete checks which missed case with `Opaque4` node between `If` and `Bool` nodes. As result we hit assert in `ConnectionGraph::specialize_cmp()` when trying to access inputs of `Cmp` node
> 
> Node* curr_cmp = curr_ctrl->in(0)->in(1)->in(1); // true/false -> if -> bool -> cmp
> con = curr_cmp->in(1)->is_Con() ? curr_cmp->in(1) : curr_cmp->in(2);
> 
> 
> This code was introduce for ReduceAllocationMerges optimization [JDK-8316991](https://bugs.openjdk.org/browse/JDK-8316991) in JDK-23. There was followup fix [JDK-8330853](https://bugs.openjdk.org/browse/JDK-8330853) to add more checks but it missed this case.
> 
> I added more strict check in `can_reduce_check_users()` to catch such cases. And added asserts in `specialize_cmp()` to verify expected nodes.
> 
> Tested tier1-3.hs-stress,hs-xcomp

Otherwise, looks good! Were you also able to reproduce this in mainline?

src/hotspot/share/opto/escape.cpp line 581:

> 579:             Node* iff_cmp = iff->in(1)->in(1);
> 580:             int opc = iff_cmp->Opcode();
> 581:             can_reduce = (opc == Op_CmpP || opc == Op_CmpN) && can_reduce_cmp(n, iff_cmp);

Can you add a comment here that we could have an `Opaque4` node for which we want to bail out?

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19921#pullrequestreview-2144527141
PR Review Comment: https://git.openjdk.org/jdk/pull/19921#discussion_r1656627530