RFR: 8290432: C2 compilation fails with assert(node->_last_del == _last) failed: must have deleted the edge just produced

[Yi Yang]

On Thu, 4 Aug 2022 17:27:35 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Hi, can I have a review for this patch? [JDK-8273585](https://bugs.openjdk.org/browse/JDK-8273585) recognized the form of `Phi->CastII->AddI` as additional parallel induction variables. In the following program:
>> 
>> class Test {
>>     static int dontInline() {
>>         return 0;
>>     }
>> 
>>     static long test(int val, boolean b) {
>>         long ret = 0;
>>         long dArr[] = new long[100];
>>         for (int i = 15; 293 > i; ++i) {
>>             ret = val;
>>             int j = 1;
>>             while (++j < 6) {
>>                 int k = (val--);
>>                 for (long l = i; 1 > l; ) {
>>                     if (k != 0) {
>>                         ret += dontInline();
>>                     }
>>                 }
>>                 if (b) {
>>                     break;
>>                 }
>>             }
>>         }
>>         return ret;
>>     }
>> 
>>     public static void main(String[] args) {
>>         for (int i = 0; i < 1000; i++) {
>>             test(0, false);
>>         }
>>     }
>> }
>> 
>> `val` is incorrectly matched with the new parallel IV form:
>> ![image](https://user-images.githubusercontent.com/5010047/182059398-fc5204bc-8d95-4e3e-8c66-15776af457b8.png)
>> And C2 further replaces it with newly added nodes, which finally leads the crash:
>> ![image](https://user-images.githubusercontent.com/5010047/182059498-13148d46-b10f-4e18-b84a-f6b9f626ac7b.png)
>> 
>> I think we can add more constraints to the new form. The form of `Phi->CastXX->AddX` appears when using Preconditions.checkIndex, and it would be recognized as additional IV when 1) Phi != phi2, 2) CastXX is controlled by RangeCheck(to reflect changes in Preconditions checkindex intrinsic)
>
> From your test I assume internal loop `for (long l=` is processed. But `val` is updated outside it. Its `AddI` node should not be part of loop's body. Could you explain?

Hi @vnkozlov, thanks for your review. Here is the (almost) complete IR before replacing additional parallel IVs:
![image](https://user-images.githubusercontent.com/5010047/183040379-3699dcc2-92a3-49ae-9181-32d526c51c75.png)

Before that, some loop optimizations are applied: `[15,293)` and `[2,6)` are recognized as counted loops, and the latter loop is maximally unrolled. Loop unswitching is applied when C2 saw `if (b)`. The program looks like:

static long test(int val, boolean b) {
    long ret = 0;
    long dArr[] = new long[100];
    if (b) {
        for (int i = 15; 293 > i; ++i) {
            ret = val;
            val--;
        }
    } else {
        for (int i = 15; 293 > i; ++i) {
            ret = val;
            val-=4
        }
    }
    return ret;
}

Their corresponding IR are as follows:
![image](https://user-images.githubusercontent.com/5010047/183042749-e492bac7-08e8-4b5d-beab-975db63db16c.png)

`Phi#564` and `Phi#125` represent `ret` in two unswitched loops, respectively. `val` is actually part of loop body.

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

PR: https://git.openjdk.org/jdk/pull/9695