[jdk17u-dev] RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v5]

[Aleksey Shipilev]

On Tue, 27 Jun 2023 04:00:30 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> The RISC-V port was originally developed at Huawei Technologies, then integrated into OpenJDK 19.
>> 
>> The 17u version of the port has continued to be maintained in the openjdk/riscv-port-jdk17u repo later
>> and has been tested for several months. There are few changes to shared HotSpot code (mostly the main
>> one is C1 conditional move/branch support for RISC-V). As required by 17u maintainer, changes to shared
>> code has been kept to a minimum. Only enabling shared changes are incorporated and these changes are
>> properly guarded with macro RISCV. So this 17u port should not breaking existing code and, although it is
>> a large patch, finally integrating it into 17u upstream should be low risk.
>> 
>> Testing on linux-riscv64 platform:
>> - [x] Bootcycle (release & fastdebug build)
>> - [x] Tier1-4 tests (release build)
>> - [x] Benchmark workloads (Dacapo, SPECJbb2015, SPECJVM2008, Renaissance. 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 ten additional commits since the last revision:
> 
>  - Merge branch 'master' into jep422-17u-backport
>  - 8285630: Fix a configure error in RISC-V cross build
>  - Adds back commit ef86ea2842b1a204834291d9d6665bfcd7b75fbc
>  - Merge branch 'master' into jep422-17u-backport
>  - Merge branch 'master' into jep422-17u-backport
>  - 8305236: Some LoadLoad barriers in the interpreter are unnecessary after JDK-8220051
>  - Merge remote-tracking branch 'upstream/master' into jep422-17u-backport
>  - Merge remote-tracking branch 'upstream/master' into jep422-17u-backport
>  - 8276799: Implementation of JEP 422: Linux/RISC-V Port

I looked at shared code changes, and they seem like a good compromise to me.
A few minor nits, though:

src/hotspot/share/c1/c1_LIR.cpp line 1752:

> 1750:      case lir_ucmp_fd2i:             s = "ucomp_fd2i";    break;
> 1751:      case lir_cmp_fd2i:              s = "comp_fd2i";     break;
> 1752:      // cmove is a LIR_Op4 on RISCV

This comment feels redundant here.

src/hotspot/share/c1/c1_LIR.hpp line 1695:

> 1693: };
> 1694: 
> 1695: #ifdef RISCV

Would you be able not to move this code? Leave it in place, so that diff is cleaner?

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

PR Review: https://git.openjdk.org/jdk17u-dev/pull/1427#pullrequestreview-1500274457
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/1427#discussion_r1243368670
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/1427#discussion_r1243355102