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