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

[Tobias Hartmann]

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