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

[Fei Yang]

On Mon, 17 Jan 2022 03:09:25 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.
>> 
>> Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
>> 
>> Thanks,
>> Xiaolin
>
> Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits:
> 
>  - Add an assertion for patchable jals' alignment
>  - Polish comments: remove the 'RVC:' prefix
>  - Polish `nmethod_entry_barrier` and RISC-V CAS related comments
>  - Remove `nmethod_entry_barrier`-related things
>  - Split c_ldsp and c_fldsp
>  - Remove useless and polish comments
>  - 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
>  - ... and 10 more: https://git.openjdk.java.net/riscv-port/compare/f26f83ff...37899306

Still some suggestions about the comments, otherwise looks good.

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

> 2028: // 2. RVC instructions in Assembler always begin with 'c_' prefix, as 'c_li',
> 2029: //    but most of time we have no need to explicitly use these instructions.
> 2030: // 3. We introduce 'CompressibleRegion' to hint instructions in this Region's RTTI range

Suggestion:

// 3. 'CompressibleRegion' is introduced to hint instructions in this Region's RTTI range

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

> 2029: //    but most of time we have no need to explicitly use these instructions.
> 2030: // 3. We introduce 'CompressibleRegion' to hint instructions in this Region's RTTI range
> 2031: //    are qualified to change to their 2-byte versions.

//    are qualified to be compressed with their 2-byte versions.

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

> 2036: //
> 2037: // 4. Using -XX:PrintAssemblyOptions=no-aliases could print RVC instructions instead of
> 2038: //    normal ones.

Suggestion:

// 4. Using -XX:PrintAssemblyOptions=no-aliases could distinguish RVC instructions from
//    normal ones.

src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1328:

> 1326:   // With RVC a call may get 2-byte aligned.
> 1327:   //   the address of jal itself (which will be patched later) should not span the cache line.
> 1328:   //   See CallStaticJavaDirectNode::compute_padding() for more info.

Suggestion:
  // With RVC a call instruction (which will be patched later) may get 2-byte aligned and could
  // span multiple cache lines. See CallStaticJavaDirectNode::compute_padding() for more info.

src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1352:

> 1350: void LIR_Assembler::emit_static_call_stub() {
> 1351:   address call_pc = __ pc();
> 1352:   assert((__ offset() % 4) == 0, "call pc (patchable jals) must be aligned to maintain atomicity");

Suggestion:
assert((__ offset() % 4) == 0, "call sites must be properly aligned");

src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 241:

> 239:   }
> 240: 
> 241:   // RISCV's amoswap instructions need a 4-byte alignment for the 4-byte word it swaps in memory

Suggestion:
 // RISCV's amoswap instructions require that the memory address must be naturally aligned.

src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 262:

> 260:   __ bind(guard);
> 261: 
> 262:   assert(__ offset() % 4 == 0, "RISCV CAS needs a 4-byte alignment for the 4-byte word it swaps in memory");

Suggestion:
assert(__ offset() % 4 == 0, "bad alignment");

src/hotspot/cpu/riscv/riscv.ad line 1197:

> 1195: //   Patching this unaligned address will make the write operation not atomic.
> 1196: //   Other threads may be running the same piece of code at full speed, causing concurrency issues.
> 1197: //   So we must ensure that it does not span a cache line so that it can be patched.

Suggestion:

// With RVC a call site may get 2-byte aligned.
// The offset encoding in jal instruction bits [12, 31] could span multiple cache lines.
// Patching this jal instruction will not be atomic when its address is not naturally aligned.
// Other threads may be running the same piece of code at the same time, thus causing concurrency issues.

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

Changes requested by fyang (Lead).

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