RFR: 8310268: RISC-V: misaligned memory access in String.Compare intrinsic [v3]

Fri Jul 21 12:27:41 UTC 2023

On Thu, 20 Jul 2023 08:31:56 GMT, Vladimir Kempik <vkempik at openjdk.org> wrote:

>> Please review this fix. it eliminates misaligned loads in String.compare on risc-v
>> 
>> for small compares ( <= 72 bytes), the instrinsic in c2_MacroAssembler.cpp is used,
>> it reads ( in case of UU/LL) 8 bytes per loop, and at then end, it reads tail - misaligned load of last 8 bytes from the string.
>> 
>> so if string length is not 8x bytes long then last load is misaligned, also it performs read/compare of some data which already was processed.
>> 
>> I have changed that to compare only last length%8 bytes using SHORT_STRING part of intrinsic for UL/LU. But for UU/LL I have made an optimised version.
>> 
>> Thanks to optimisations for conditional branching at line [947](https://github.com/openjdk/jdk/pull/14534/files#diff-35eb1d2f1e2f0514dd46bd7fbad49ff2c87703d5a3041a6433956df00a3fe6e6R947) I’ve got no perf drop on thead ( with +AvoidUnalignedAccesses) which supports misaligned access.
>> 
>> Improvements to inflate_XX() methods gives 3-5% improvements for UL/LU cases on thead, almost no perf change on hifive.
>> 
>> for large strings, the instrinsics from stubGenerator.cpp is used
>> for UU/LL - generate_compare_long_string_same_encoding, I have just replaced misaligned ld with load_long_misaligned. Since this tail reading is not on hot path, this give some small penalty for thead when -XX:+AvoidUnalignedAccesses.
>> 
>> large LU/UL comparision is done in compare_long_string_different_encoding in sutbGenerator.cpp:
>> These changes are partially based on feilongjiang's patch, but I have changed tail reading to prevent reading past the end of string. I have observed no perf difference between feilongjiang's and my version.
>> 
>> This also enables regression test for string.Compare which previously was aarch64-only
>> 
>> Testing: tier1 and tier2 clean on hifive.
>> 
>> JMH testing, hifive:
>> before:
>> 
>> Benchmark                                   (delta)  (size)  Mode  Cnt     Score    Error  Units
>> StringCompareToDifferentLength.compareToLL        2      24  avgt    9     6.474 ±  1.475  ms/op
>> StringCompareToDifferentLength.compareToLL        2      36  avgt    9   125.823 ±  1.947  ms/op
>> StringCompareToDifferentLength.compareToLL        2      72  avgt    9    10.512 ±  0.236  ms/op
>> StringCompareToDifferentLength.compareToLL        2     128  avgt    9    13.032 ±  0.821  ms/op
>> StringCompareToDifferentLength.compareToLL        2     256  avgt    9    18.983 ±  0.318  ms/op
>> StringCompareToDifferentLength.compareToLL        2     512  avgt    9    29.925 ± ...
>
> Vladimir Kempik has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Simplify case for long LU UL compares

Updated results on hifive:

Benchmark                                   (delta)  (size)  Mode  Cnt    Score   Error  Units
StringCompareToDifferentLength.compareToLL        2      24  avgt    9    8.694 ± 0.920  ms/op
StringCompareToDifferentLength.compareToLL        2      36  avgt    9    9.583 ± 0.840  ms/op
StringCompareToDifferentLength.compareToLL        2      72  avgt    9   11.671 ± 0.710  ms/op
StringCompareToDifferentLength.compareToLL        2     128  avgt    9   15.860 ± 0.980  ms/op
StringCompareToDifferentLength.compareToLL        2     256  avgt    9   21.407 ± 0.509  ms/op
StringCompareToDifferentLength.compareToLL        2     512  avgt    9   32.435 ± 0.622  ms/op
StringCompareToDifferentLength.compareToLL        2     520  avgt    9   30.888 ± 0.402  ms/op
StringCompareToDifferentLength.compareToLL        2     523  avgt    9   33.265 ± 0.489  ms/op
StringCompareToDifferentLength.compareToLU        2      24  avgt    9   14.335 ± 0.182  ms/op
StringCompareToDifferentLength.compareToLU        2      36  avgt    9   18.220 ± 0.360  ms/op
StringCompareToDifferentLength.compareToLU        2      72  avgt    9   32.331 ± 1.166  ms/op
StringCompareToDifferentLength.compareToLU        2     128  avgt    9   50.462 ± 1.495  ms/op
StringCompareToDifferentLength.compareToLU        2     256  avgt    9   91.972 ± 1.635  ms/op
StringCompareToDifferentLength.compareToLU        2     512  avgt    9  174.093 ± 0.384  ms/op
StringCompareToDifferentLength.compareToLU        2     520  avgt    9  178.736 ± 2.627  ms/op
StringCompareToDifferentLength.compareToLU        2     523  avgt    9  183.112 ± 2.090  ms/op
StringCompareToDifferentLength.compareToUL        2      24  avgt    9   16.344 ± 0.486  ms/op
StringCompareToDifferentLength.compareToUL        2      36  avgt    9   19.724 ± 0.628  ms/op
StringCompareToDifferentLength.compareToUL        2      72  avgt    9   34.062 ± 0.513  ms/op
StringCompareToDifferentLength.compareToUL        2     128  avgt    9   52.019 ± 1.212  ms/op
StringCompareToDifferentLength.compareToUL        2     256  avgt    9   93.111 ± 1.267  ms/op
StringCompareToDifferentLength.compareToUL        2     512  avgt    9  176.734 ± 0.695  ms/op
StringCompareToDifferentLength.compareToUL        2     520  avgt    9  179.531 ± 1.215  ms/op
StringCompareToDifferentLength.compareToUL        2     523  avgt    9  184.424 ± 1.069  ms/op
StringCompareToDifferentLength.compareToUU        2      24  avgt    9    9.966 ± 0.648  ms/op
StringCompareToDifferentLength.compareToUU        2      36  avgt    9   11.915 ± 0.662  ms/op
StringCompareToDifferentLength.compareToUU        2      72  avgt    9   15.422 ± 0.925  ms/op
StringCompareToDifferentLength.compareToUU        2     128  avgt    9   19.726 ± 0.581  ms/op
StringCompareToDifferentLength.compareToUU        2     256  avgt    9   31.103 ± 0.738  ms/op
StringCompareToDifferentLength.compareToUU        2     512  avgt    9   51.847 ± 0.861  ms/op
StringCompareToDifferentLength.compareToUU        2     520  avgt    9   53.083 ± 0.326  ms/op
StringCompareToDifferentLength.compareToUU        2     523  avgt    9   53.209 ± 0.477  ms/op

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

PR Comment: https://git.openjdk.org/jdk/pull/14534#issuecomment-1645501190