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

[Mikhail Ablakatov]

On Thu, 19 Sep 2024 14:04:52 GMT, Mikhail Ablakatov <duke at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 2877:
>> 
>>> 2875:     f(0b01111, 28, 24);                                                                            \
>>> 2876:     if (T == T4H || T == T8H) {                                                                    \
>>> 2877:       f(0b01, 23, 22), f(index & 0b11, 21, 20), rf(Vm, 16), f(op2, 15, 12), f(index >> 2 & 1, 11); \
>> 
>> This isn't right.
>> Please go to test/hotspot/gtest/aarch64/aarch64-asmtest.py and add `mulv` to the set of tested instructions. Please make sure you test all modes.
>
> Could you point me towards asm tests for *Vector - Scalar* insts like  `fmlavs`, `fmlsvs` or `fmulxvs` ([assembler_aarch64.hpp#L2857](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/assembler_aarch64.hpp:2857))? Judging by git blame these were added without any new tests. Am I right assuming we are missing asm tests for *Vector - Scalar* instructions in general?

It was renamed to `mulvs` by https://github.com/openjdk/jdk/pull/18487/commits/419f39473b53099b7bd42c33380a6ccb3917ab16

>> 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.

Removed by https://github.com/openjdk/jdk/pull/18487/commits/9f4ae8554106d3e1290ee127b848d7f48952e5fb .

>> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 5465:
>> 
>>> 5463:       __ addv(vmul0, load_arrangement, vmul0, vdata0);
>>> 5464:     } else if (load_arrangement == Assembler::T8B || load_arrangement == Assembler::T4H ||
>>> 5465:                load_arrangement == Assembler::T8H) {
>> 
>> Use a switch here, and everywhere else that a switch applies.
>
> The only other piece of code similar to this one is the one at line [5591](https://github.com/openjdk/jdk/pull/18487/files#diff-9112056f732229b18fec48fb0b20a3fe824de49d0abd41fbdb4202cfe70ad114R5591). Any other which I'm missing?

Fixed by https://github.com/openjdk/jdk/pull/18487/commits/a824a74263ce9309fca50a90b3d0f09e9b56a5c4 , please check.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18487#discussion_r1767000130
PR Review Comment: https://git.openjdk.org/jdk/pull/18487#discussion_r1766997201
PR Review Comment: https://git.openjdk.org/jdk/pull/18487#discussion_r1766998899