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

[Roland Westrelin]

On Mon, 13 May 2024 13:23:46 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> In the test case:
>> 
>> 
>> long i;
>> for (; i > 0; i--) {
>>     res += 42 / ((int) i);
>> 
>> 
>> The long counted loop phi has type `[1..100]`. As a consequence, the
>> `ConvL2I` also has type `[1..100]`. The `DivI` node that follows can't
>> fault: it is not guarded by a zero check and has no control set.
>> 
>> The `ConvL2I` is split through phi and so is the `DiVI` node:
>> `PhaseIdealLoop::cannot_split_division()` returns true because the
>> value coming from the backedge into the `DivI` (when it is about to be
>> split thru phi) is the result of the `ConvL2I` which has type
>> `[1..100`] so is not zero as far as the compiler can tell.
>> 
>> On the last iteration of the loop, i is 1. Because the DivI was split
>> thru Phi, it computes the value for the following iteration, so for i
>> = 0. This causes a crash when the compiled code runs.
>> 
>> The same problem can't happen with an int counted loop because logic
>> in `PhaseIdealLoop::split_thru_phi()` prevents a `ConvI2L` from being
>> split thru phi. I propose to fix this the same way: in the test case,
>> it's not true that once the `ConvL2I` is split thru phi it keeps type
>> `[1..100]`. The fix is fairly conservative because it's base on the
>> existing logic for `ConvI2L`: we would want to not split a `ConvL2I`
>> only a counted loopd but. I suppose the same is true for the `ConvI2L`
>> and I thought it would be best to revisit both together.
>
> Roland Westrelin has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - test case tweaks
>  - fuzzer test

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;
}

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

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