RFR: 8317720: RISC-V: Implement Adler32 intrinsic

[Fei Yang]

On Tue, 19 Mar 2024 17:03:26 GMT, ArsenyBochkarev <duke at openjdk.org> wrote:

> Hello everyone! Please review this non-vectorized implementation of `_updateBytesAdler32` intrinsic. Reference implementation for AArch64 can be found [here](https://github.com/openjdk/jdk9/blob/master/hotspot/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp#L3281).
> 
> ### Correctness checks
> 
> Test `test/hotspot/jtreg/compiler/intrinsics/zip/TestAdler32.java` is ok. All tier1 also passed.
> 
> ### Performance results on T-Head board
> 
> Enabled intrinsic:
> 
> | Benchmark                          |    (count) |  Mode |  Cnt  |   Score  |  Error |  Units |
> | ------------------------------------- | ----------- | ------ | --------- | ------ | --------- | ---------- |
> | Adler32.TestAdler32.testAdler32Update |      64 | thrpt  | 25 | 5522.693 | 23.387 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |     128 | thrpt |  25 | 3430.761 |  9.210 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |     256 | thrpt |  25 | 1962.888 |  5.323 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |     512 | thrpt  | 25 | 1050.938 |  0.144 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |    1024 | thrpt  | 25 |  549.227 |  0.375 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |    2048 | thrpt  | 25 |  280.829 |  0.170 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |    5012 | thrpt  | 25 |  116.333 |  0.057 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |    8192 | thrpt  | 25  |  71.392 |  0.060 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |   16384 | thrpt |  25  |  35.784 |  0.019 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update |   32768 | thrpt |  25  |  17.924 |  0.010 | ops/ms |
> | Adler32.TestAdler32.testAdler32Update  |  65536 | thrpt |  25  |   8.940 |  0.003 | ops/ms |
> 
> Disabled intrinsic:
> 
> | Benchmark                          |    (count) |  Mode |  Cnt  |   Score  |  Error |  Units |
> | ------------------------------------- | ----------- | ------ | --------- | ------ | --------- | ---------- |
> |Adler32.TestAdler32.testAdler32Update|64|thrpt|25|655.633|5.845|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|128|thrpt|25|587.418|10.062|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|256|thrpt|25|546.675|11.598|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|512|thrpt|25|432.328|11.517|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|1024|thrpt|25|311.771|4.238|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|2048|thrpt|25|202.648|2.486|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|5012|thrpt|25|100.246|1.119|ops/ms|
> |Adler32.TestAdler32.testAdler32Update|8192|thr...

Sorry for the late reply. Some comments after a brief look. BTW: Could you please merge master to retrigger the GHA? Thanks.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 5086:

> 5084:     const uint64_t BASE = 0xfff1;
> 5085:     const uint64_t NMAX = 0x15B0;
> 5086: 

I think it's better to start a new frame on stub enter and exit with `__ enter()` and `__ leave()` respectively for proper stackwalking of RuntimeStub frame.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 5160:

> 5158:     __ srli(temp2, temp0, 56);
> 5159:     __ add(s1, s1, temp2);
> 5160:     __ add(s2, s2, s1);

I see a lot of duplicate logic in this function. Can we factor out some common logic as separate functions? Like generate_updateBytesAdler32_accum_16, generate_updateBytesAdler32_accum_8, etc.

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

PR Review: https://git.openjdk.org/jdk/pull/18382#pullrequestreview-1967697752
PR Review Comment: https://git.openjdk.org/jdk/pull/18382#discussion_r1543964079
PR Review Comment: https://git.openjdk.org/jdk/pull/18382#discussion_r1543958995