RFR: 8308504: C2: "malformed control flow" after JDK-8303466

[Emanuel Peter]

On Thu, 22 Jun 2023 12:26:06 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>>> It doesn't seem to be true that the loop incr never overflows in the general case. See this example:
>> 
>> But don't we check that the limit is small enough at runtime, such that there cannot be an overflow? We do that with `check_stride_overflow` and `insert_loop_limit_check_predicate`. And if it does overflow, we do not go into the counted-loop, but we uncommon trap, I think.
>> Or are you sure that we actually enter the counted-loop with your example? Or just some peeled version?
>> 
>>> Would that be required? I was hoping that the CastII on the limit before unrolling would cause the phi type to be narrow
>> enough.
>> 
>> Ok, let's think this through for going from pre-loop to main loop / zero-trip-guard of the main loop:
>> 
>> `incr = iv + stride`
>> exit check: `iv + stride = incr < limit`
>> 
>> So that the incr does not overflow, it would have to have type `init+stride ... max_int` (`stride > 0`). That means we would want the phi to have a type that has `stride` subtracted from `hi`, hence `init..max_int-stride`. For `stride < 0` we want the incr to have type `min_int...init+stride` and the `phi` should have `min_int-stride ... init`.
>> 
>> So the issue in the example above is that the pre-loop phi has type `minint...7`, and not `minint+3...7`, if I understand you correctly, @rwestrel . Because with that type, the `incr` would then have type `min_int..4`, and that would let the zero-trip-guard recognize that it is always false, since the limit has type `8...maxint`.
>> 
>> `PhiNode::Value` determines the range for counted-loops. We take the limit into account there. But it seems we don't do the right thing there yet. I'll have a look into that, and if a `CastII` on the limit helps.
>> 
>> Now what happens for the main-loop to post-loop? There, the stride has changed with the unrolling. So maybe we'd have to somehow do more to ensure the incr of the main-loop cannot overflow. I guess that could maybe be an issue for the vectorized drain-loop for vector-super-unrolling, or the post-loop.
>
>> > It doesn't seem to be true that the loop incr never overflows in the general case. See this example:
>> 
>> But don't we check that the limit is small enough at runtime, such that there cannot be an overflow? We do that with `check_stride_overflow` and `insert_loop_limit_check_predicate`. And if it does overflow, we do not go into the counted-loop, but we uncommon trap, I think. Or are you sure that we actually enter the counted-loop with your example? Or just some peeled version?
> 
> Ran with:
> 
>  -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:-TieredCompilation -XX:CompileOnly=TestOverflowCountedLoopIncr::test -XX:CompileCommand=quiet -XX:LoopMaxUnroll=0 -XX:+UseCountedLoopSafepoints 
> 
> I see a single counted loop, no uncommon trap in the IR.

@rwestrel so you think the `incr` can indeed overflow, and that is ok? Or would that be a bug? Why do we even have the loop limit check in the first place, if overflow is allowed?

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

PR Comment: https://git.openjdk.org/jdk/pull/14331#issuecomment-1602562601