RFR(S): 8189067: SuperWord optimization crashes with "assert(out == prev || prev == __null) failed: no branches off of store slice"
tobias.hartmann at oracle.com
Fri Oct 13 06:08:56 UTC 2017
thanks for the review!
On 12.10.2017 19:33, Vladimir Kozlov wrote:
> Good. I think we should leave this conservative fix without optimizing it. We should not spend a lot of time optimizing
> C2 now.
Okay, that's fine with me. I anyone wants to optimize that later, he/she can file an RFE.
> On 10/12/17 3:04 AM, Tobias Hartmann wrote:
>> please review the following patch:
>> The problem is in the C2 optimization that moves stores out of a loop .
>> 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
>> 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).
>>  https://bugs.openjdk.java.net/browse/JDK-8080289
More information about the hotspot-compiler-dev