RFR: 8304948: [vectorapi] C2 crashes when expanding VectorBox [v2]

Eric Liu eliu at openjdk.org
Wed Apr 19 08:11:54 UTC 2023


> This patch fixes C2 failure with SIGSEGV due to endless recursion.
> 
> With test case VectorBoxExpandTest.java in this patch, C2 would generate IR graph like below:
> 
> 
>                ------------
>               /            \
>       Region |  VectorBox   |
>            \ | /            |
>             Phi             |
>              |              |
>              |              |
>       Region |  VectorBox   |
>            \ | /            |
>             Phi             |
>              |              |
>              |------------/
>              |
> 
> 
> 
> This Phi will be optimized by merge_through_phi [1], which transforms `Phi (VectorBox VectorBox)` into `VectorBox (Phi Phi)` to pursue opportunity of combining VectorBox with VectorUnbox. In this process, either the pre type check [2] or the process cloning Phi nodes [3], the circle case is well considered to avoid falling into endless loop.
> 
> After merge_through_phi, each input Phi of new VectorBox has the same shape with original root Phi before merging (only VectorBox has been replaced). After several other optimizations, C2 would expand VectorBox [4] on a graph like below:
> 
> 
>                ------------
>               /            \
>       Region |  Proj        |
>            \ | /            |
>             Phi             |
>              |              |
>              |              |
>       Region |  Proj        |
>            \ | /            |
>             Phi             |
>              |              |
>              |------------/
>              |
>              |     Phi
>              |     /
>           VectorBox
> 
> 
> which the circle case should be taken into consideration as well.
> 
> [TEST]
> Full Jtreg passed without new failure.
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L2554
> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L2571
> [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L2531
> [4] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/vector.cpp#L311

Eric Liu 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 three additional commits since the last revision:

 - Merge jdk:master
   
   Change-Id: I63c06b87d5b0c20ddaec0aa43031872b8ebb5362
 - fix typo
   
   Change-Id: I1b84c4957398178bf234f71242a1cdd044181a79
 - 8304948: [vectorapi] C2 crashes when expanding VectorBox
   
   This patch fixes C2 failure with SIGSEGV due to endless recursion.
   
   With test case VectorBoxExpandTest.java in this patch, C2 would generate
   IR graph like below:
   
   ```
                  ------------
                 /            \
         Region |  VectorBox   |
              \ | /            |
               Phi             |
                |              |
                |              |
         Region |  VectorBox   |
              \ | /            |
               Phi             |
                |              |
                |\------------/
                |
   
   ```
   
   This Phi will be optimized by merge_through_phi [1], which transforms
   `Phi (VectorBox VectorBox)` into `VectorBox (Phi Phi)` to pursue
   opportunity of combining VectorBox with VectorUnbox. In this process,
   either the pre type check [2] or the process cloning Phi nodes [3], the
   circle case is well considered to avoid falling into endless loop.
   
   After merge_through_phi, each input Phi of new VectorBox has the same
   shape with original root Phi before merging (only VectorBox has been
   replaced). After several other optimizations, C2 would expand VectorBox
   [4] on a graph like below:
   
   ```
                  ------------
                 /            \
         Region |  Proj        |
              \ | /            |
               Phi             |
                |              |
                |              |
         Region |  Proj        |
              \ | /            |
               Phi             |
                |              |
                |\------------/
                |
                |     Phi
                |     /
             VectorBox
   
   ```
   which the circle case should be taken into consideration as well.
   
   [TEST]
   Full Jtreg passed without new failure.
   
   [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L2557
   [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L2574
   [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/cfgnode.cpp#L2534
   [4] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/vector.cpp#L316
   
   Change-Id: I381b1ba7e0865814d97535e365db6d9d72ef1949

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/13489/files
  - new: https://git.openjdk.org/jdk/pull/13489/files/0e73688a..7119ed69

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

  Stats: 245814 lines in 2038 files changed: 221344 ins; 11953 del; 12517 mod
  Patch: https://git.openjdk.org/jdk/pull/13489.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13489/head:pull/13489

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


More information about the hotspot-compiler-dev mailing list