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

Tobias Hartmann tobias.hartmann at oracle.com
Thu Oct 12 14:24:42 UTC 2017

I forgot to mention that my fix also re-enables UseSubwordForMaxVector which was disabled due to JDK-8184995 [1] which 
turned out to be a duplicate of this issue and is not caused by UseSubwordForMaxVector.

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

On 12.10.2017 12:04, Tobias Hartmann wrote:
> 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

More information about the hotspot-compiler-dev mailing list