RFR: 8349479: C2: when a Type node becomes dead, make CFG path that uses it unreachable [v3]
Roland Westrelin
roland at openjdk.org
Fri Mar 28 15:32:30 UTC 2025
> This is primarily motivated by 8275202 (C2: optimize out more
> redundant conditions). In the following code snippet:
>
>
> int[] array = new int[arraySize];
> if (j <= arraySize) {
> if (i >= 0) {
> if (i < j) {
> int v = array[i];
>
>
> (`arraySize` is a constant)
>
> at the range check, `j` is known to be in `[min, arraySize]` as a
> consequence, `i` is known to be `[0, arraySize-1]`. The range check
> can be eliminated.
>
> Now, if later, `i` constant folds to some value that's positive but
> out of range for the array:
>
> - if that happens when the new pass runs, then it can prove that:
>
> if (i < j) {
>
> is never taken.
>
> - if that happens during IGVN or CCP however, that condition is not
> constant folded. And because the range check was removed, there's no
> guard protecting the range check `CastII`. It becomes `top` and, as
> a result, the graph can become broken.
>
> What I propose here is that when the `CastII` becomes dead, any CFG
> paths that use the `CastII` node is made unreachable. So in pseudo code:
>
>
> int[] array = new int[arraySize];
> if (j <= arraySize) {
> if (i >= 0) {
> if (i < j) {
> halt();
>
>
> Finding the CFG paths is implemented in the patch by following the
> uses of the node until a CFG node or a `Phi` is encountered.
>
> The patch applies this to all `Type` nodes as with 8275202, I also ran
> in some rare corner cases with other types of nodes. The exception is
> `Phi` nodes which may not be as easy to handle (and for which I had no
> issue with 8275202).
>
> Finally, the patch includes a test case that's unrelated to the
> discussion of 8275202 above. In that test case, a `CastII` becomes top
> but the test that guards it doesn't constant fold. The root cause is a
> transformation of:
>
>
> (CastII (AddI
>
>
> into
>
>
> (AddI (CastII ) (CastII)`
>
>
> which causes the resulting node to have a wider type. The `CastII`
> captures a type before the transformation above happens. Once it has
> happened, the guard for the `CastII` can't be constant folded when an
> out of bound value occurs.
>
> This is likely fixable some other way (eventhough it doesn't seem
> straightforward). Given the long history of similar issues (and the
> test case that shows that they are more hiding), I think it would
> make sense to try some other way of approaching them.
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 five additional commits since the last revision:
- review
- Merge branch 'master' into JDK-8349479
- review
- whitespace
- fix & test
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/23468/files
- new: https://git.openjdk.org/jdk/pull/23468/files/73dd6d84..f310865f
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=23468&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=23468&range=01-02
Stats: 177870 lines in 4403 files changed: 75946 ins; 76167 del; 25757 mod
Patch: https://git.openjdk.org/jdk/pull/23468.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/23468/head:pull/23468
PR: https://git.openjdk.org/jdk/pull/23468
More information about the hotspot-compiler-dev
mailing list