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

[Vladimir Kozlov]

On Thu, 27 May 2021 21:05:33 GMT, Jamsheed Mohammed C M <jcm at openjdk.org> wrote:

>> Issue is similar to https://bugs.openjdk.java.net/browse/JDK-8261730
>> but happens at next https://github.com/jamsheedcm/jdk/blob/master/src/hotspot/share/opto/gcm.cpp#L830
>> 
>> Request for review
>
> 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.

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

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