RFR: 8265132 : C2 compilation fails with assert "missing precedence edge" [v4]

[Jamsheed Mohammed C M]

On Fri, 25 Jun 2021 22:39:14 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Jamsheed Mohammed C M has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   improving the fix for raise_LCA_above_marks
>
> Both cases 8261730 and 8265132 have the same code pattern:
> 
> 
>    foo(M a, M b) {
>      if (a == null) uncommon_trap();
>      int v = a.val;
>      if (b == null) uncommon_trap(v); // v is local we need to pass for deoptimization
>      b.val = v;
>      b.val = 0;
>    }
> 
> 
> First, we have optimization to fold `Back-to-back stores`:
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/memnode.cpp#L2657
> After that the only use of `v` is debug info in uncommon trap
> 
> Second, `a` and `b` could be the same object in general case. Loads and store are truly dependent and we have to preserve their order in such case. But it is not in our cases because `a` is not `null` and `b` is `null` based on checks in code when we go into second uncommon trap path. 8265132 case is more complicated since object is passed in array.
> 
> 
>   foo(M a, M b) {
>      if (a == null) uncommon_trap();
>      if (b == null) {
>        int v = a.val;
>        uncommon_trap(v); // v is local we need to pass for deoptimization
>      }
>      // folded: b.val = v;
>      b.val = 0;
>   }
> 
> 
> And after `implicit_null_check` optimization we got:
> 
> 
>   foo(M a, M b) {
>      if (a == null) uncommon_trap();
>      if (null_check(b.val = 0)) {
>        int v = a.val;
>        uncommon_trap(v); // v is local we need to pass for deoptimization
>      }
>   }
> 
> 
> We could move load about store's implicit null check to avoid hitting assert but it will cause performance issue because we will execute load without using its result.
> 
> Based on this the final code is correct for our cases and we need to relax asserts as we did in 8261730 case.
> 
> I need to look more on Jamsheed extra changes to see if we need them.

Thank you @vnkozlov @dean-long @veresov for the reviews and help.

jdk 17 PR => https://github.com/openjdk/jdk17/pull/191

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

PR: https://git.openjdk.java.net/jdk/pull/4200