[riscv-port] RFR: 8278994: riscv: RVC support [v10]

Xiaolin Zheng xlinzheng at openjdk.java.net
Mon Jan 10 03:04:40 UTC 2022


On Sat, 8 Jan 2022 11:09:59 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
>> 
>>  - Move RVC code to the proper location after rebasing (#42)
>>  - Rename misc functions and change the positions of some comments
>>  - Remove remaining macros as discussions
>>  - Remain an 'minimum_alignment' unchanged
>>  - Manually inline all macros into functions as discussions
>>  - Remove assembler_riscv_c.hpp as discussions
>>  - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions
>>  - Fix remaining CEXT -> RVC
>>  - Remove Alignment-related changes as discussions
>>  - Update licenses to the new year
>>  - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151
>
> src/hotspot/cpu/riscv/assembler_riscv.hpp line 2053:
> 
>> 2051: 
>> 2052:   // RVC: extract a 16-bit instruction.
>> 2053:   static inline uint16_t c_extract(uint16_t val, unsigned msb, unsigned lsb) {
> 
> Looks like c_extract and c_sextract are not used?

Thanks for pointing out this - this seems to be inherited from the original patch (the version that we decode instructions in `emit()`) and they seem to be not used anymore and I have removed them.

> src/hotspot/cpu/riscv/riscv.ad line 1194:
> 
>> 1192: 
>> 1193: // RVC: With RVC a call may get 2-byte aligned.
>> 1194: //   The offset encoding in jal ranges bits [12, 31], which could span the cache line.
> 
> Maybe rephrasing like this:
> //   The offset encoding in jal ranges bits [12, 31] could span the cache line.

done. Thanks for polishing the sentence.

> src/hotspot/cpu/riscv/riscv.ad line 1204:
> 
>> 1202: }
>> 1203: 
>> 1204: // RVC: With RVC a call may get 2-byte aligned.
> 
> Looks like this comment dupliates the one for CallStaticJavaDirectNode::compute_padding.

done.

> src/hotspot/cpu/riscv/vm_version_riscv.cpp line 109:
> 
>> 107:   }
>> 108: 
>> 109:   // compressed instruction extension
> 
> I think this should better to place this one after UseRVV and UseRVB.

done. Thanks for pointing out this. It is inherited from my original patch and it indeed needs to be moved downward to keep the consistency.

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

PR: https://git.openjdk.java.net/riscv-port/pull/34


More information about the riscv-port-dev mailing list