[10] RFR(S): 8189067: SuperWord optimization crashes with "assert(out == prev || prev == __null) failed: no branches off of store slice"

[Tobias Hartmann]

Hi,

please review the following patch:
https://bugs.openjdk.java.net/browse/JDK-8189067
http://cr.openjdk.java.net/~thartmann/8189067/webrev.01/

The problem is in the C2 optimization that moves stores out of a loop [1].

We move stores out of a loop by creating clones at all loop exit paths that observe the stored value. When walking up 
the dominator chain from observers of a store and placing clones of the store at the top (right after the loop), we may 
end up placing multiple cloned stores at the same location. This confuses/crashes the SuperWord optimization and also 
affects performance of the generated code due to double execution of the same store (see details in the bug comments).

My initial prototype fix (webrev.00) just checked if there already is a cloned store with the same control input and 
reused it instead of creating another one. I've discussed this with Roland in detail and we came to the conclusion that 
this is not sufficient. We may still end up with a broken memory graph: If one use of the store is a load, we create a 
clone of the store and connect it to the load but this store is not connected to the rest of the memory graph, i.e. the 
memory effect of the store is not propagated. Although this may not cause incorrect execution (at least we were not able 
to trigger that), it may cause problems if other optimizations kick in and in some cases we still end up with the same 
store being executed twice.

We agreed to go with the safer version of computing the LCA and move the store there (only the initial store without 
creating clones) if it's outside of the loop. This is a bit too strict in cases where there's an uncommon trap in the 
loop (see 'TestMoveStoresOutOfLoops::test_after_6') but it works fine in the other cases exercised by the test. If 
people agree, I'll file a follow up RFE to improve the optimization but would like to go with the safe fix for now (this 
also affects JDK 9).

Thanks,
Tobias

[1] https://bugs.openjdk.java.net/browse/JDK-8080289