RFR: 8286125: C2: "bad AD file" with PopulateIndex on x86_64 [v2]
Pengfei Li
pli at openjdk.java.net
Tue May 10 05:09:44 UTC 2022
> A fuzzer test reports an assertion failure issue with PopulateIndexNode
> on x86_64. It can be reproduced by the new jtreg case inside this patch.
> Root cause is that C2 superword creates a PopulateIndexNode by mistake
> while vectorizing below loop.
>
> for (int i = 304; i > 15; i -= 3) {
> int c = 16;
> do {
> for (int t = 1; t < 1; t++) {}
> arr[c + 1] >>= i;
> } while (--c > 0);
> }
>
> This is a corner loop case with redundant code inside. After several C2
> optimizations, the do-while loop inside is unrolled and then isomorphic
> right shift statements can be combined in the superword optimization.
> Since all shift counts are the same loop IV value `i`, superword should
> generate a RShiftCntVNode to create a vector of scalar replications of
> the loop IV. But after JDK-8280510, a PopulateIndexNode is generated by
> mistake because of the `opd == iv()` condition.
>
> To fix this, we add a `have_same_inputs` condition here checking if all
> inputs at position `opd_idx` of nodes in the pack are the same. If true,
> C2 code should NOT run into this block to generate a PopulateIndexNode.
> Instead, it should run into the next block for scalar replications.
>
> Additionally, only adding this condition here is still not good enough
> because it breaks the experimental post loop vectorization. As in post
> loops, all packs are singleton, i.e., `have_same_inputs` is always true.
> Hence, we also add a pack size check here to make post loop logic run
> into this block. It's safe to let it go because post loop never needs
> scalar replications of the loop IV - it never combines nodes in packs.
>
> We also add two more assertions in the code.
>
> Jtreg hotspot::hotspot_all_no_apps, jdk::tier1~3 and langtools::tier1
> are tested and no issue is found.
Pengfei Li 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 two additional commits since the last revision:
- Merge branch 'master' into slpfix
- 8286125: C2: "bad AD file" with PopulateIndex on x86_64
A fuzzer test reports an assertion failure issue with PopulateIndexNode
on x86_64. It can be reproduced by the new jtreg case inside this patch.
Root cause is that C2 superword creates a PopulateIndexNode by mistake
while vectorizing below loop.
for (int i = 304; i > 15; i -= 3) {
int c = 16;
do {
for (int t = 1; t < 1; t++) {}
arr[c + 1] >>= i;
} while (--c > 0);
}
This is a corner loop case with redundant code inside. After several C2
optimizations, the do-while loop inside is unrolled and then isomorphic
right shift statements can be combined in the superword optimization.
Since all shift counts are the same loop IV value `i`, superword should
generate a RShiftCntVNode to create a vector of scalar replications of
the loop IV. But after JDK-8280510, a PopulateIndexNode is generated by
mistake because of the `opd == iv()` condition.
To fix this, we add a `have_same_inputs` condition here checking if all
inputs at position `opd_idx` of nodes in the pack are the same. If true,
C2 code should NOT run into this block to generate a PopulateIndexNode.
Instead, it should run into the next block for scalar replications.
Additionally, only adding this condition here is still not good enough
because it breaks the experimental post loop vectorization. As in post
loops, all packs are singleton, i.e., `have_same_inputs` is always true.
Hence, we also add a pack size check here to make post loop logic run
into this block. It's safe to let it go because post loop never needs
scalar replications of the loop IV - it never combines nodes in packs.
We also add two more assertions in the code.
Jtreg hotspot::hotspot_all_no_apps, jdk::tier1~3 and langtools::tier1
are tested and no issue is found.
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8587/files
- new: https://git.openjdk.java.net/jdk/pull/8587/files/e91eb0bf..3af03a80
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8587&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8587&range=00-01
Stats: 101523 lines in 1186 files changed: 92290 ins; 4348 del; 4885 mod
Patch: https://git.openjdk.java.net/jdk/pull/8587.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8587/head:pull/8587
PR: https://git.openjdk.java.net/jdk/pull/8587
More information about the hotspot-compiler-dev
mailing list