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