RFR: 8271954: C2: assert(false) failed: Bad graph detected in build_loop_late [v4]

[Christian Hagedorn]

On Thu, 26 Aug 2021 15:28:19 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add set_ctrl
>
> src/hotspot/share/opto/loopPredicate.cpp line 192:
> 
>> 190:   assert(rgn->in(rgn->req() -1) == if_uct, "new edge should be last");
>> 191:   bool has_phi = false;
>> 192:   bool unswitch_clone_for_fast_loop = old_new != NULL && uncommon_proj->outcnt() > 1;
> 
> Isn't it inconsistent that you compute unswitch_clone_for_fast_loop here but pass a precomputed is_slow_loop when the expression looks almost identical: why compute one locally but pass the other one as an argument? Or am I missing something?

I want to pass a tristate: [no unswitching, fast loop, slow loop]. I initially had two bool flags passed to this function for that. But then I thought since `old_new` is only non-null in the fast loop case, I can use that one as "flag" to decide if we need to clone for the fast loop. I use `unswitch_is_slow_loop` as flag for the slow loop since `old_new` is `NULL` in this case. `false` and `NULL` is then the third state for no unswitching. But maybe it's better to introduce the second bool flag again to make it more clear.

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

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