RFR: 8311862: RISC-V: small improvements to shift immediate instructions [v3]

Fei Yang fyang at openjdk.org
Fri Jul 14 13:49:59 UTC 2023


On Fri, 14 Jul 2023 07:32:49 GMT, Ilya Gavrilin <duke at openjdk.org> wrote:

>> Please review this small change for slli srli and srai
>> slli change allows to replace slli Rd, Rs, 0 with addi Rd, Rs, 0 (and no operation emited if Rd == Rs)
>> addi with 0 has higher chances to be just a register renaming and not utilise ALU at all
>> We have observed small positive effect on hifive (and no change on thead).
>> 
>> testing: tier1 and tier2 on hifive, also hotspot/jtreg/compiler/intrinsics/string tests on Qemu with UseZba
>> 
>> performance on hifive, before:
>> |              Benchmark              | Mode | Cnt |   Score  |   |  Error  | Units |
>> |:-----------------------------------:|:----:|:---:|:--------:|:-:|:-------:|:-----:|
>> | StringIndexOf.advancedWithShortSub1 | avgt |  25 | 4035.143 | ± | 191.262 | ns/op |
>> | StringIndexOf.advancedWithShortSub2 | avgt |  25 | 1145.807 | ± |  14.610 | ns/op |
>> 
>> with patch:
>> |              Benchmark              | Mode | Cnt |   Score  |   |  Error  | Units |
>> |:-----------------------------------:|:----:|:---:|:--------:|:-:|:-------:|:-----:|
>> | StringIndexOf.advancedWithShortSub1 | avgt |  25 | 3613.943 | ± | 178.153 | ns/op |
>> | StringIndexOf.advancedWithShortSub2 | avgt |  25 | 923.169 | ± |  47.123| ns/op |
>
> Ilya Gavrilin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert changes related with slli_uw

Two nits remain. Looks good otherwise.

src/hotspot/cpu/riscv/assembler_riscv.hpp line 2793:

> 2791:       _slli(Rd, Rs1, shamt);                                                                 \
> 2792:     } else {                                                                                 \
> 2793:       if(Rd != Rs1) {                                                                        \

Nit: s/if(Rd != Rs1) {/if (Rd != Rs1) {/

src/hotspot/cpu/riscv/assembler_riscv.hpp line 2814:

> 2812:       NORMAL_NAME(Rd, Rs1, shamt);                                                           \
> 2813:     } else {                                                                                 \
> 2814:       if(Rd != Rs1) {                                                                        \

Nit: s/if(Rd != Rs1) {/if (Rd != Rs1) {/

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

Marked as reviewed by fyang (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14823#pullrequestreview-1530340814
PR Review Comment: https://git.openjdk.org/jdk/pull/14823#discussion_r1263757303
PR Review Comment: https://git.openjdk.org/jdk/pull/14823#discussion_r1263757495


More information about the hotspot-dev mailing list