RFR: 8331575: C2: crash when ConvL2I is split thru phi at LongCountedLoop [v2]

[Emanuel Peter]

On Tue, 14 May 2024 08:08:08 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Before split if:
>> 
>> long i = 100;
>> for (; i > 0;) {
>>   // i here is 1..100
>>   int j = (int)i; // ConvL2I type is 1..100, same as loop phi
>>   int k = 42 / j;
>>    i--;
>> }
>> 
>> 
>> after split if:
>> 
>> 
>> long i = 100;
>> int j = 100;
>> int k = 0;
>> for (; i > 0;) {
>>   // i here is 1..100
>>    i--;
>>   // i here is 0..99
>>   j = (int)i; // ConvL2I type is still 1..100 which is not correct
>>   k = 42 / j;
>> }
>
>> @rwestrel which "split_if" optimization was applied in your example? Split the ConvI2L through the phi? If so, the problem seems to be that the ConvI2L floats by the exit-check, right?
> 
> Yes.
> 
>> So I guess that is really a limitation: a trip count `Phi` specifically does the narrowing, and so you cannot just split past it. The question is if that is really nice, or if we could do it differently, e.g. via a `CastLL/CastII` on the exit-check?
> 
> The issue involves conv nodes when split thru phi at a counted loop. That's a narrow corner case. I think fixing it by addressing the corner case where it occurs as proposed is simpler than trying a most general fix which can have hard to anticipate consequences.

@rwestrel Yes, I'm totally fine with the fix. It simply applies the `int` case to `long`.

In a future RFE, we could at least restrict the "bailout" to trip-count Phi's, and not all Phi's.

In even further RFE's, we could consider doing the type narrowing not in the trip-count phi, but via casts at the checks. That would be a more unified solution. Generally, I feel like we are struggling way too much with all the different ways one can pin and narrow types: it is all mixed into trip-count phi's, Cast's, Conv's etc. Who really can understand all the complicated interactions? It seem we keep piling on special-case logic, but it is a endless whack-a-mole game. Every fix is "simple" but the sum of all those fixes is far from "simple" ;)

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

PR Comment: https://git.openjdk.org/jdk/pull/19086#issuecomment-2109575708