RFR: 8295407: C2 crash: Error: ShouldNotReachHere() in multiple vector tests with -XX:-MonomorphicArrayCheck -XX:-UncommonNullCast [v2]

[Fei Gao]

On Wed, 16 Nov 2022 06:04:09 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Fei Gao 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:
>> 
>>  - Clean up related code
>>  - Merge branch 'master' into fg8295407
>>  - 8295407: C2 crash: Error: ShouldNotReachHere() in multiple vector tests with -XX:-MonomorphicArrayCheck -XX:-UncommonNullCast
>>    
>>    For unsupported `CMove` patterns, JDK-8293833 helps remove unused
>>    `CMove` and related packs from superword candidate packset by the
>>    function `remove_cmove_and_related_packs()`, but it only works
>>    when `-XX:+UseVectorCmov` is enabled[1]. When the option is not
>>    enabled, these unsupported `CMove` packs are still kept in the
>>    superword packset, causing the same failure.
>>    
>>    Actually, the function `filter_packs()` in superword is to filter
>>    out unsupported packs but it can't work as expected currently for
>>    these `CMove` cases. As we know, not all `CMove` packs can be
>>    vectorized. `merge_packs_to_cmovd()`[2] looks through all packs
>>    in the superword packset and generates a `CMove` candidate
>>    packset to collect all qualified `CMove` packs. Hence, only
>>    `CMove` packs in the `CMove` candidate packset are our target
>>    patterns and can be vectorized. But `filter_packs()` thinks,
>>    if the `CMove` pack is in a superword packset and its vector
>>    node is implemented in the current platform, then it can
>>    be vectorized. Therefore, the function doesn't remove
>>    these unsupported packs.
>>    
>>    We can adjust the function `implemented()` in the stage of
>>    `filter_packs()` to check if the current `CMove` pack is in
>>    the `CMove` candidate packset. If not, `filter_packs()` considers
>>    it not to be vectorized and then remove it. After the fix,
>>    whether `-XX:+UseVectorCmov` is enabled or not, these
>>    unsupported packs can be removed by `filter_packs()`. In this
>>    way, we don't need the function`remove_cmove_and_related_packs()`
>>    anymore and thus the patch also cleans related code.
>>    
>>    [1] https://github.com/openjdk/jdk/blob/9b9be88bcaa35c89b6915ff0c251e5a04b10b330/src/hotspot/share/opto/superword.cpp#L537
>>    [2] https://github.com/openjdk/jdk/blob/9b9be88bcaa35c89b6915ff0c251e5a04b10b330/src/hotspot/share/opto/superword.cpp#L1892
>>    [3] https://github.com/openjdk/jdk/blob/9b9be88bcaa35c89b6915ff0c251e5a04b10b330/src/hotspot/share/opto/superword.cpp#L2701
>
> Looks good to me. Let's wait for Vladimir's testing to finish.

Thanks for your kind review and test work, @TobiHartmann @vnkozlov. I'll integrate it.

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

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