RFR: 8261308: C2: assert(inner->is_valid_counted_loop(T_INT) && inner->is_strip_mined()) failed: OuterStripMinedLoop should have been removed

[Roland Westrelin]

On Tue, 23 Feb 2021 19:13:22 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> The inner counted loop of the test case starts at 1 and stops at 1 so
>> runs for one iteration. A counted loop is created for it. The iv Phi
>> is found to be the constant 1 and its type is set by:
>> 
>> l->phi()->as_Phi()->set_type(l->phi()->Value(&_igvn));
>> 
>> in PhaseIdealLoop::is_counted_loop() but it's not replaced by the
>> constant 1 yet so the counted loop's shape is preserved.
>> 
>> IdealLoopTree::do_one_iteration_loop() runs but doesn't optimize the
>> loop because the trip count is not set to 1. The loop contains a range
>> check and range check elimination is applied. That causes the loop
>> exit test to be adjusted with a MinI(..) expression. When IGVN runs
>> next, the phi is replaced with 1 but because the exit test was
>> changed, IGVN can't prove it always fails. So the loop is not removed
>> which causes the assert failure as loop opts progress.
>> 
>> The fix I propose is for IdealLoopTree::do_one_iteration_loop() to
>> remove the 1 iteration loop. The reason it doesn't happen is that
>> IdealLoopTree::compute_trip_count() doesn't set the trip count because
>> it finds a zero trip count: limit - init = 1 - 1 = 0. All loops, once
>> entered execute at least once. So I think, it's safe to set the trip
>> count to 1 in those cases.
>
> src/hotspot/share/opto/loopTransform.cpp line 126:
> 
>> 124:     jlong limit_con = (stride_con > 0) ? limit_type->_hi : limit_type->_lo;
>> 125:     int stride_m = stride_con - (stride_con > 0 ? 1 : -1);
>> 126:     jlong trip_count = (limit_con - init_con + stride_m)/stride_con;
> 
> Does it mean that before we execute do_one_iteration_loop() optimization for loops which do 2 trips?
> This seems wrong. Can you check?
> I agree that loop body executed at least once since it is exit check. But it means we have to generally adjust `trip_count` with `+1` and not do what you suggested.

Thanks for the comments.
A CountedLoopNode/CountedLoopEndNode is this in pseudo code:
int i = init;
do {
  i += stride;
} while (i < limit);
for stride > 0
We always enter the loop body so execute it at least once.
If limit > init, it's executed: (limit - init) / stride times if (limit - init) is a multiple of stride. 
if (limit - init) is not a multiple of stride then it's executed (limit - init) / stride + 1.
That matches the formula in the source code AFAICT.
In what case do you think we compute 1 for the trip count when it's actually 2?
How would you compute the trip count?

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

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