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

[Eric Liu]

On Wed, 19 Apr 2023 08:14:46 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> 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
>
> Another review would be good.

Thanks your kindly review. @TobiHartmann @merykitty @jatin-bhateja

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

PR Comment: https://git.openjdk.org/jdk/pull/13489#issuecomment-1535559681