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