RFR: 8291599: Assertion in PhaseIdealLoop::skeleton_predicate_has_opaque after JDK-8289127 [v3]

Roland Westrelin roland at openjdk.org
Fri Sep 9 12:05:03 UTC 2022


> In TestPhiInSkeletonPredicateExpression.test1():
> 
> - Loop predication adds predicates for the null check of array and the
>   array range check. It also adds skeleton predicates in case of
>   subsequent unrolling.
> 
> - One of the skeleton predicate has the following shape:
> 
> (Opaque4 (Bool (CmpUL (AddL (AddL (ConvI2L (LoadI (Phi ...))) (ConvI2L (CastII (AddI (OpaqueLoopInit OpaqueLoopStride))))) -1) ...)))
> 
> - Split thru phi pushes the null check through the dominating
>   region. The skeleton predicate subgraph is transformed to:
> 
> (Opaque4 (Bool (CmpUL (Phi ...) ...)))
> 
> - Logic that processes skeleton predicate can no longer find the
>   OpaqueLoopInit and OpaqueLoopStride nodes because they are now
>   behind a phi. That causes the assert to fire.
> 
> The fix I propose is to catch cases where part of a skeleton predicate
> expression (a subgraph with a OpaqueLoopInit or OpaqueLoopStride node)
> is being split during split if and to clone the entire skeleton
> predicate subgraph then.
> 
> There's a already logic for that currently but it only triggers if
> PhaseIdealLoop::split_up() tries to split an OpaqueLoopInit or
> OpaqueLoopStride. In the case here, the OpaqueLoopInit and
> OpaqueLoopStride nodes have control above the region at which split if
> occurs. So they are never split by PhaseIdealLoop::split_up(). The
> AddL nodes in subgraph are.

Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:

 - Christian's review
 - Merge branch 'master' into JDK-8291599
 - Update test/hotspot/jtreg/compiler/loopopts/TestPhiInSkeletonPredicateExpression.java
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - Update src/hotspot/share/opto/loopTransform.cpp
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - fix
 - test

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/10022/files
  - new: https://git.openjdk.org/jdk/pull/10022/files/a54f0ee7..db4bbedb

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10022&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10022&range=01-02

  Stats: 50079 lines in 1080 files changed: 19346 ins; 21470 del; 9263 mod
  Patch: https://git.openjdk.org/jdk/pull/10022.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10022/head:pull/10022

PR: https://git.openjdk.org/jdk/pull/10022


More information about the hotspot-compiler-dev mailing list