RFR: 8314124: RISC-V: implement Base64 intrinsic - decoding [v4]

[Fei Yang]

On Fri, 23 Aug 2024 08:45:38 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> ## Performance
>> benchmarks run on CanVM-K230
>> 
>> data
>> <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 m2+m1+scalar | (addSpecial) | (errorIndex) | (lineSize) | (maxNumBytes) | Mode | Cnt | Score +intrinsic+rvv | Score -intrinsic | Error | Units | Improvement
>> -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | --
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 1 | avgt | 10 | 97.771 | 98.506 | 0.713 | ns/op | 1.008
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 3 | avgt | 10 | 117.715 | 118.422 | 0.428 | ns/op | 1.006
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 7 | avgt | 10 | 174.625 | 172.767 | 7.671 | ns/op | 0.989
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 32 | avgt | 10 | 286.391 | 317.175 | 11.443 | ns/op | 1.107
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 64 | avgt | 10 | 336.932 | 503.257 | 15.738 | ns/op | 1.494
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 80 | avgt | 10 | 418.894 | 625.485 | 7.21 | ns/op | 1.493
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 96 | avgt | 10 | 353.813 | 698.67 | 15.485 | ns/op | 1.975
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 112 | avgt | 10 | 499.243 | 866.909 | 4.427 | ns/op | 1.736
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 512 | avgt | 10 | 1451.277 | 3530.048 | 3.685 | ns/op | 2.432
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 1000 | avgt | 10 | 2258.785 | 5964.066 | 9.075 | ns/op | 2.64
>> Base64Decode.testBase64Decode | 0 | 144 | 4 | 20000 | avgt | 10 | 39689.204 | 122334.929 | 255.195 | ns/op | 3.082
>> Base64Decode.testBase64MIMEDecode | 0 | 144 | 4 | 1 | avgt | 10 | 187.032 | 158.558 | 7.606 | ns/op | 0.848
>> Base64Decode.testBase64MIMEDecode | 0 | 144 | 4 | 3 | avgt | 10 | 209.558 | 200.774 | 7.648 | ns/op | 0.958
>> Base64Decode.testBase64MIMEDecode | 0 | 144 | 4 | 7 | avgt | 10 | 556.696 | 505.072 | 8.748 | ns/op | 0.907
>> Base64Decode.testBase64MIMEDecode | 0 | 144 | 4 | 32 | avgt | 10 | 2139.767 | 1876.825 | 13.787 | ns/op | 0.877
>> Base64Decode.testBase64MIMEDecode | 0 | 144 | 4 | 64 | avgt | 10 | 6142.353 | 3818.199 | 35.622 | ns/op | 0.622
>> Base64Decode.testBase64MIMEDecode | 0 | 144 | 4 | 80 | avgt | 10 | 8746.205 | 4787.155 | 109.819 | ns/op ...
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comments

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

> 5395:    *  c_rarg4   - dp, dst start offset
> 5396:    *  c_rarg5   - isURL, Base64 or URL character set
> 5397:    *  c_rarg6   - isMIME, Decoding MIME block - unused here

Seems "unused here" in the code comment is not accurate? As I see `isMIME` is checked in the code.

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

> 5458: 
> 5459:     // passed in length (send - soff) is guaranteed to be > 4,
> 5460:     // and in this intrinsic we only processe data of length in multiple of 4,

Nit: s/processe/process/

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

> 5474:     __ BIND(ProcessData);
> 5475: 
> 5476: 

Nit: I think a single empty line is enough here.

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

> 5490:       }
> 5491: 
> 5492:       __ BIND(ScalarLoop);

Why not move this `ScalarLoop` body to immediately before `Exit`? Seems to me that we will have a more clear separation of scalar code and vector code and a more simpler control flow then.

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

> 5537:     }
> 5538: 
> 5539: 

Similar here. A single empty line is enough.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20026#discussion_r1728640550
PR Review Comment: https://git.openjdk.org/jdk/pull/20026#discussion_r1728628017
PR Review Comment: https://git.openjdk.org/jdk/pull/20026#discussion_r1729789588
PR Review Comment: https://git.openjdk.org/jdk/pull/20026#discussion_r1729794073
PR Review Comment: https://git.openjdk.org/jdk/pull/20026#discussion_r1729789889