RFR: 8286301: Port JEP 425 to RISC-V [v4]

Richard Reingruber rrich at openjdk.org
Fri Nov 4 10:16:12 UTC 2022


On Fri, 4 Nov 2022 06:27:39 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hi,
>> 
>> Please review this PR porting JEP 425 (Virtual Threads) to RISC-V.
>> 
>> This is mainly adapted from the work of AArch64 port. Most of the changes lie in RISC-V scope.
>> Changes to HotSpot shared code are trivial and are always guarded by RISCV64 macro. So this won't
>> affect the rest of the world in theory.
>> 
>> There exists some differences in frame structure between AArch64 and RISC-V.
>> For AArch64, we have:
>> 
>> enum {
>>   link_offset           = 0,
>>   return_addr_offset    = 1,
>>   sender_sp_offset      = 2
>> };
>> 
>> While for RISC-V, we have:
>> 
>> enum {
>>   link_offset           = -2,
>>   return_addr_offset    = -1,
>>   sender_sp_offset      =  0
>> };
>> 
>> So we need adapations in some places where the code relies on value of sender_sp_offset to work.
>> Note that implementation for Post-call NOPs optimization is not incorporated in this PR as we plan to
>> evaluate more on its impact on performance.
>> 
>> Testing on Linux-riscv64 HiFive Unmatched board:
>>   - Minimal, Client and Server release & fastdebug build OK.
>>   - Passed tier1-tier4 tests (release build).
>>   - Passed jtreg tests under test/jdk/java/lang/Thread/virtual with extra JVM options: -XX:+VerifyContinuations -XX:+VerifyStack (fastdebug build).
>>   - Performed benchmark tests like Dacapo, SPECjvm2008, SPECjbb2015, etc. to make sure no performance regression are introduced (release build).
>
> Fei Yang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Code cleanup
>  - Merge branch 'master' into 8286301
>  - Merge branch 'master' into 8286301
>  - Fix
>  - 8286301: JEP 425 to RISC-V

Shared changes look good.
Maybe we can get rid of the RISCV64 dependency when allocating the CodeBuffer for the enter intrinsic.

Thanks, Richard.

src/hotspot/share/oops/stackChunkOop.inline.hpp line 139:

> 137: bool stackChunkOopDesc::is_usable_in_chunk(void* p) const {
> 138: #if (defined(X86) || defined(AARCH64) || defined(RISCV64)) && !defined(ZERO)
> 139:   HeapWord* start = (HeapWord*)start_address() + sp() - frame::sender_sp_offset RISCV64_ONLY(- frame::metadata_words);

I think this RISCV64 special case will go away with the following change form the PPC64 port: https://github.com/openjdk/jdk/pull/10961/files#diff-563a75c6d20d6f6f35362f5afef7d4f061b9536a37f7c95c27ac37879246d899R138-R139

src/hotspot/share/runtime/sharedRuntime.cpp line 3123:

> 3121: 
> 3122:       if (method->is_continuation_enter_intrinsic()) {
> 3123:         buffer.initialize_stubs_size(NOT_RISCV64(128) RISCV64_ONLY(192));

Can we use the maximum over all platforms? This is a temporary buffer anyway. The generated code, constants, etc. is copied to the nmethod. See https://github.com/openjdk/jdk/blob/8ee0f7d5982d95674cfc1b217dbabaeafefbc8f1/src/hotspot/share/code/nmethod.cpp#L659-L660

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

Changes requested by rrich (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10917


More information about the hotspot-dev mailing list