[15] RFR(XS): 8239852:java/util/concurrent tests fail with +VerifyGraphEdges: assert(!VerifyGraphEdges) failed: verification should have failed

[Nils Eliasson]

+1

// Nils

On 2020-02-28 12:30, Tobias Hartmann wrote:
> Hi Christian,
>
> looks reasonable to me.
>
> Best regards,
> Tobias
>
> On 26.02.20 12:49, Christian Hagedorn wrote:
>> Hi
>>
>> Please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8239852
>> http://cr.openjdk.java.net/~chagedorn/8239852/webrev.00/
>>
>> The failing assert is too strong in some cases where the IR is valid but PhaseCFG::schedule_late()
>> bails out at [1] and the recompilation with subsume_loads == false does not bail out anymore.
>>
>> One such case occurs during the compilation of
>> java.util.concurrent.locks.AbstractQueuedLongSynchronizer::hasQueuedPredecessors when running, for
>> example, [2] with -XX:+VerifyGraphEdges. Before calling PhaseCFG::insert_anti_dependences(), LCA and
>> early both point to block A with _dom_depth x for some node. However, the early block is temporarily
>> changed inside insert_anti_dependences() by memory_early_block() to point to an earlier block B with
>> _dom_depth y < x. The LCA is updated to point to block B and is returned. As a result, the assert
>> fails because early->_dom_depth (x) > LCA->_dom_depth (y). The recompilation of the method with
>> subsume_loads == false does not bail out anymore.
>>
>> The assert can be completely remove since we already have this assert [3] which catches an
>> unschedulable graph of a compilation with subsume_loads == false and there are no other bailouts in
>> PhaseCFG::schedule_late().
>>
>>
>> Thank you!
>>
>> Best regards,
>> Christian
>>
>>
>> [1] http://hg.openjdk.java.net/jdk/jdk/file/61ee15b9a1ac/src/hotspot/share/opto/gcm.cpp#l1349
>> [2]
>> http://hg.openjdk.java.net/jdk/jdk/file/61ee15b9a1ac/test/jdk/java/util/concurrent/Semaphore/RacingReleases.java
>>
>> [3] http://hg.openjdk.java.net/jdk/jdk/file/61ee15b9a1ac/src/hotspot/share/opto/gcm.cpp#l1352