RFR: 8342692: C2: long counted loop/long range checks: don't create loop-nest for short running loops [v32]

[Emanuel Peter]

On Tue, 27 May 2025 07:11:12 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review
>
> src/hotspot/share/opto/loopnode.cpp line 1178:
> 
>> 1176: // - CountedLoop: Can be reused.
>> 1177: bool PhaseIdealLoop::short_running_loop(IdealLoopTree* loop, jint stride_con, const Node_List &range_checks,
>> 1178:                                         uint iters_limit) {
> 
> Are only long-loops allowed, or can there be int loops here too?
> You talk about long range checks, which makes me believe this is about long loops. But then below you check for the `bt` of the head, so could that be int?
> 
> If it is about longs only: it would be nice to have an assert, and I would also rename the method to include the `long`.
> 
> You may also want to say what the return `bool` means (success, the loop was indeed a short running (long?) loop).

Hmm, or does the `int` `bt` come from `StressLongCountedLoop`, which complicates things here?

> src/hotspot/share/opto/loopnode.cpp line 1188:
> 
>> 1186:   loop->compute_trip_count(this, bt);
>> 1187:   // Loop must run for no more than iter_limits as it guarantees no overflow of scale * iv in long range checks.
>> 1188:   // iters_limit / ABS(stride_con) is the largest trip count for which we know it's correct to not create a loop nest:
> 
> Can you point to somewhere that argues this in more detail? Especially the thing about overflows sounds important.

Where are the assumptions about what is needed for correctness coming from?

> src/hotspot/share/opto/loopnode.cpp line 1255:
> 
>> 1253:     assert(short_running_loop_predicate_proj->in(0)->is_ParsePredicate(), "must be parse predicate");
>> 1254: 
>> 1255:     jlong limit_long = iters_limit;
> 
> Suggestion:
> 
>     const jlong iters_limit_long = iters_limit;

To keep name consistency.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2108393380
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2108383543
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2108428465