RFR: 8260650: test failed with "assert(false) failed: infinite loop in PhaseIterGVN::optimize"

[Tobias Hartmann]

On Tue, 16 Mar 2021 05:08:15 GMT, Igor Veresov <iveresov at openjdk.org> wrote:

> The test constructs a large tree of `AddINode`s that add constants to a variable. The tree is completely constant-foldable. Depending of the processing order the folding process may happen in the loop in `PhaseIterGVN::transform_old()` instead of going through the worklist. That blows through the loop limit, which triggers assert. The solution is to make the loop limit proportional to the number of live nodes. I chose it to be `K*live_nodes()`, which is already the limit in `PhaseIterGVN::optimize()`. I also noticed the same problem in `PhaseGVN::transform_no_reclaim()` and adjusted the code accordingly.

Changes requested by thartmann (Reviewer).

src/hotspot/share/opto/phaseX.cpp line 857:

> 855:     i = apply_ideal(k, /*can_reshape=*/false);
> 856:   }
> 857:   NOT_PRODUCT(if(loop_count != 0) { set_progress(); })

Newline after `if` is missing.

src/hotspot/share/opto/phaseX.cpp line 844:

> 842:   // Apply the Ideal call in a loop until it no longer applies
> 843:   Node *k = n;
> 844:   Node *i = apply_ideal(k, /*can_reshape=*/false);

You might want to fix the asterisk position (`Node *i` vs. `Node* i`) while you are at it (also in the method signature).

src/hotspot/share/opto/phaseX.cpp line 851:

> 849:     NOT_PRODUCT(loop_count++;)
> 850: #ifdef ASSERT
> 851:     if (loop_count >= K * C->live_nodes()) {

With old code, `loop_count` would be `0` in the first iteration (due to the postfix increment), with your changes it is `2`. Is that intended? I see that you've changed `<` to `>=` but I still find it counter-intuitive that the `loop_count` is not the actual count. Also, it's inconsistent with `PhaseIterGVN::transform` where you increment after the check.

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

PR: https://git.openjdk.java.net/jdk/pull/3022