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

[Tobias Hartmann]

On Sat, 6 Aug 2022 07:42:18 GMT, Yi Yang <yyang 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)
>
> Yi Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update comment

Could you explain why it's an issue in this case if the control input of the `CastII` is **not** a range check?

src/hotspot/share/opto/loopnode.cpp line 3688:

> 3686:     if (incr2->in(1)->is_ConstraintCast() && !incr2->in(1)->in(0)->is_RangeCheck()) {
> 3687:       // Skip AddI->CastII->Phi case if CastII is not controlled by local RangeCheck
> 3688:       // to reflect changes in LibraryCallKit::inline_preconditions_checkIndex

In the valid case, isn't the ConstraintCast control input `incr2->in(1)->in(0)` the IfTrue projection of the RangeCheck?

I would remove the second line because it's not clear which "changes" in `LibraryCallKit::inline_preconditions_checkIndex` it is referring to.

Suggestion:

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

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