RFR: 8322770: Implement C2 VectorizedHashCode on AArch64 [v9]

[Mikhail Ablakatov]

On Wed, 18 Sep 2024 12:01:25 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Mikhail Ablakatov 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 10 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8322770
>>  - cleanup: adjust a comment in the light of the latest change
>>  - cleanup: fix comment formatting
>>    
>>    Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>>  - Optimize both the stub and inlined parts of the implementation
>>    
>>    Process T_CHAR/T_SHORT elements using T8H arrangement instead of T4H.
>>    Add a non-unrolled vectorized loop to the stub to handle vectorizable
>>    tail portions of arrays multiple to 4/8 elements (for ints / other
>>    types). Make the stub process array as a whole instead of relying on
>>    the inlined part to process an unvectorizable tail.
>>  - cleanup: add comments and simplify the orr ins
>>  - cleanup: remove redundant copyright notice
>>  - cleanup: use a constexpr function for intpow instead of a templated class
>>  - cleanup: address review comments
>>  - cleanup: remove a redundant parameter
>>  - 8322770: AArch64: C2: Implement VectorizedHashCode
>>    
>>    The code to calculate a hash code consists of two parts: a stub method that
>>    implements a vectorized loop using Neon instruction which processes 16 or 32
>>    elements per iteration depending on the data type; and an unrolled inlined
>>    scalar loop that processes remaining tail elements.
>>    
>>    [Performance]
>>    
>>    [[Neoverse V2]]
>>    ```
>>                                                                |  328a053 (master) |  dc2909f (this)  |
>>    ----------------------------------------------------------------------------------------------------------
>>      Benchmark                               (size)  Mode  Cnt |    Score    Error |    Score   Error | Units
>>    ----------------------------------------------------------------------------------------------------------
>>      ArraysHashCode.bytes                         1  avgt   15 |    0.805 ?  0.206 |    0.815 ? 0.141 | ns/op
>>      ArraysHashCode.bytes                        10  avgt   15 |    4.362 ?  0.013 |    3.522 ? 0.124 | ns/op
>>      ArraysHashCode.bytes                       100  avgt   15 |   78.374 ?  0.136 |   12.935 ? 0.016 | ns/op
>>      ArraysHashCode.bytes                     10000  avgt   15 | 9247.335 ? 13.691 | 1344.770 ? 1.898 | ns/op
>>      ArraysHashCode.cha...
>
> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 5416:
> 
>> 5414:                                 : load_arrangement == Assembler::T8H ? 36  // 9 insts
>> 5415:                                 : load_arrangement == Assembler::T8B ? 40  // 10 insts
>> 5416:                                                                      : -1; // invalid
> 
> This is extremely fragile in the presence of code change. Can we not simply delete it?

There's a `guarantee()` at the end of the loop to verify the size so the code change shouldn't be left unnoticed.

> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 5563:
> 
>> 5561:     }
>> 5562: 
>> 5563:     start = __ offset();
> 
> What does this logic from 5552 onwards do? It at least deserves a comment.
> Can we not simply delete it?

It's here to make sure the loop takes the smallest number of aligned 32-byte instruction memory regions possible.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18487#discussion_r1764943608
PR Review Comment: https://git.openjdk.org/jdk/pull/18487#discussion_r1764941830