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

[Igor Veresov]

On Tue, 16 Mar 2021 08:24:13 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Igor Veresov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address Tobias' comments.
>
> Changes requested by thartmann (Reviewer).

@TobiHartmann, I've hopefully addressed your comments, please take another look when you have a chance.

> 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.

You mean space? I put the space in.

> 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.

Yeah, you're right it is a little sloppy. I will straighten this out. The intent is to count optimization steps. Since in `transform_old` and `transform_no_reclaim`, the loop is peeled to have the first `apply_ideal()` happen before it, I start the loop with 1. The original code had quite a few flaws, so, don't be concerned about counting the same way, the heuristic of the limit is obviously very approximate.

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

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