RFR: 8353041: NeverBranchNode causes incorrect block frequency calculation

[Roberto Castañeda Lozano]

On Tue, 8 Apr 2025 08:39:02 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> This fixes a quality of implementation issue for infinite loops using a NeverBranch node.  We need Block::succ_prob() to return 1.0f for the 100% successful back-edge so that block frequencies are computed correctly.  I also fixed Block_Stack::most_frequent_successor() to choose the correct successor.  I verified that this corrects the huge frequency ratio that was detected and clamped by JDK-8346888.
>> 
>> Currently this bug is labeled noreg-hard with no new regression test, as it's not obvious how to write such as test.
>
> src/hotspot/share/opto/domgraph.cpp line 244:
> 
>> 242:       // edges swapped, rare case
>> 243:       succ_idx = 1;
>> 244:     } else {
> 
> Could you explain how do we end up at this rare case? In my opinion, it would be preferable, if possible, to ensure edges are never swapped in the first place.

Never mind, I found the answer in https://github.com/openjdk/jdk/pull/11481. Maybe it would be worth extending the comment with a summary of why this can happen.

Suggestion:

    if (succ == b->_succs[1]->head()) {
      // Edges swapped, rare case. May happen due to an unusual matcher
      // traversal order for peeled infinite loops.
      succ_idx = 1;
    } else {

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24390#discussion_r2032723880