[riscv-port] RFR: 8278994: riscv: RVC: basic instruction set

Fei Yang fyang at openjdk.java.net
Fri Dec 24 07:18:49 UTC 2021


On Mon, 20 Dec 2021 08:10:10 GMT, Xiaolin Zheng <xlinzheng at openjdk.org> wrote:

> Hi team,
> 
> This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
> 
> Thanks,
> Xiaolin

Changes requested by fyang (Lead).

src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 27:

> 25: 
> 26: #ifndef CPU_RISCV_ASSEMBLER_RISCV_CEXT_HPP
> 27: #define CPU_RISCV_ASSEMBLER_RISCV_CEXT_HPP

This should corresponds to the real file name

src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 30:

> 28: 
> 29: private:
> 30:   bool _in_compressible_region;

So where will this variable be used then?

src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 36:

> 34: public:
> 35: 
> 36:   // C-Ext: If an instruction is compressible, then

I would suggest we change "C-Ext" into something like "RVC".

src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 67:

> 65: 
> 66:   // C-Ext: extract a 16-bit instruction.
> 67:   static inline uint16_t c_extract(uint16_t val, unsigned msb, unsigned lsb) {

We definitely need test coverage for those newly added assember functions for this PR.

src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 571:

> 569: 
> 570: // C-Ext: an uncompressible region
> 571: class UncompressibleRegion : public AbstractCompressibleRegion {

It looks strange that UncompressibleRegion extends the abstract 'compressible' region here.

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1321:

> 1319: #ifdef ASSERT
> 1320:     tty->print_cr("pd_patch_instruction_size: instruction 0x%x at " INTPTR_FORMAT " could not be patched!\n", *(unsigned*)branch, p2i(branch));
> 1321:     Disassembler::decode(branch - 10, branch + 10);

How is this change tested?

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

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


More information about the riscv-port-dev mailing list