RFR: 8334554: RISC-V: verify & fix perf of string comparison

[Hamlin Li]

On Mon, 24 Jun 2024 06:32:26 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hi,
>> Can you help to review this patch?
>> Thanks!
>> 
>> As in compare-UL/LU, it already uses m4, so in this patch also use m4 for compare-UU/LL.
>> 
>> ## Test
>> tested on K230-CanMV, vlen = 128.
>> warmup: 10 times
>> iteration: 10 times
>> 
>> ### Before patch
>> <google-sheets-html-origin style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
>> Benchmark | (size) | Score+rvv | Score-rvv | -rvv/+rvv
>> -- | -- | -- | -- | --
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLL | 24 | 4242936.876 | 7227607.14 | 1.703444419
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLL | 36 | 5738695.363 | 8157070.353 | 1.421415468
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLL | 72 | 7163243.984 | 7209568.036 | 1.00646691
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLL | 128 | 8627566.301 | 12720927.51 | 1.474451435
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLL | 256 | 14632020.04 | 16291127.26 | 1.113388802
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLL | 512 | 26539410.59 | 23612505.95 | 0.8897147833
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLU | 24 | 4913490.894 | 10454585.94 | 2.127730807
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLU | 36 | 7230036.286 | 13561865.48 | 1.875767277
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLU | 72 | 9525418.104 | 21901656.51 | 2.299285582
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLU | 128 | 12645301.4 | 37351484.04 | 2.953783611
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLU | 256 | 21147886.68 | 64886475.43 | 3.068225039
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToLU | 512 | 39738017.94 | 125169103.6 | 3.149857745
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToUL | 24 | 5183884.427 | 11040441.7 | 2.129762314
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToUL | 36 | 7421224.1 | 13879329.16 | 1.870221
>> com.arm.benchmarks.intrinsics.StringCompareToDifferentLength.compareToUL | 72 | 9739241.916 |...
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2330:
> 
>> 2328: void C2_MacroAssembler::element_compare(Register a1, Register a2, Register result, Register cnt, Register tmp1, Register tmp2,
>> 2329:                                         VectorRegister vr1, VectorRegister vr2, VectorRegister vrs, bool islatin, Label &DONE,
>> 2330:                                         bool is_m2) {
> 
> How about add a `Assembler::LMUL LMUL` param instead? And, should we pass a larger `Assembler::m4` only for vlen=128 case (that is when `MaxVectorSize` is 16)? As I mentioned on [[1]](https://github.com/openjdk/jdk/pull/18382#discussion_r1645356197), a LMUL larger than needed can sometimes even bring a negative impact on performance on hardwares like banana-pi (vlen=256), which is kind of strange to me.
> 
> Performance impact on banana-pi (vlen=256):
> Before:
> 
> Benchmark                                   (delta)  (size)  Mode  Cnt      Score      Error  Units
> StringCompareToDifferentLength.compareToLL        2      24  avgt    9   4556.938 ±  909.960  us/op
> StringCompareToDifferentLength.compareToLL        2      36  avgt    9   4613.250 ±  891.120  us/op
> StringCompareToDifferentLength.compareToLL        2      72  avgt    9   5792.938 ±  545.470  us/op
> StringCompareToDifferentLength.compareToLL        2     128  avgt    9   5884.248 ± 1089.558  us/op
> StringCompareToDifferentLength.compareToLL        2     256  avgt    9   8506.465 ±  197.376  us/op
> StringCompareToDifferentLength.compareToLL        2     512  avgt    9  14349.963 ±  253.898  us/op
> StringCompareToDifferentLength.compareToLU        2      24  avgt    9   6084.199 ± 5148.464  us/op
> StringCompareToDifferentLength.compareToLU        2      36  avgt    9   5194.196 ±  927.611  us/op
> StringCompareToDifferentLength.compareToLU        2      72  avgt    9   7332.861 ±  909.214  us/op
> StringCompareToDifferentLength.compareToLU        2     128  avgt    9   7043.723 ±  159.843  us/op
> StringCompareToDifferentLength.compareToLU        2     256  avgt    9  11718.996 ±  552.570  us/op
> StringCompareToDifferentLength.compareToLU        2     512  avgt    9  20471.987 ±  314.224  us/op
> StringCompareToDifferentLength.compareToUL        2      24  avgt    9   5371.997 ± 1002.623  us/op
> StringCompareToDifferentLength.compareToUL        2      36  avgt    9   5469.605 ± 1119.210  us/op
> StringCompareToDifferentLength.compareToUL        2      72  avgt    9   7249.683 ±  154.028  us/op
> StringCompareToDifferentLength.compareToUL  ...

Seems the `Error` column is huge for tests `compareToLL`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19825#discussion_r1651158380