[riscv-port-jdk11u:riscv-port] RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v6]

[Xiaolin Zheng]

On Thu, 21 Sep 2023 12:38:09 GMT, kuaiwei <duke at openjdk.org> wrote:

> The test build is building what is in pull/3/merge, so it contains all three recent updates. That means there is still something missing.

Thanks for your confirmation and that helps. I see the root cause after another examination of the code. Shortly, here is the fix.


commit d57bb681fbb31a7f405c18ac9ef7e328d54e3797 (HEAD -> pull/3-fix)
Author: Xiaolin Zheng <zhengxiaolinx at gmail.com>
Date:   Thu Sep 21 22:58:59 2023 +0800

    Fix: Fixed-length mv() mistakenly redirected to li() during reshaping

    Caused by: https://github.com/openjdk/riscv-port-jdk11u/pull/3/commits/d8b14fd5e6455b47cfcb02d13c0c24c74e824570#diff-7a5c3ed05b6f3f06ed1c59f5fc2a14ec566a6a5bd1d09606115767daa99115bdL1367-L1372

diff --git a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
index f592d7585d..f851cc1e41 100644
--- a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
+++ b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
@@ -1343,6 +1343,12 @@ void MacroAssembler::mv(Register Rd, Address dest) {
   movptr(Rd, dest.target());
 }

+void MacroAssembler::mv(Register Rd, address addr) {
+  // Here in case of use with relocation, use fix length instruction
+  // movptr instead of li
+  movptr(Rd, addr);
+}
+
 void MacroAssembler::mv(Register Rd, RegisterOrConstant src) {
   if (src.is_register()) {
     mv(Rd, src.as_register());
diff --git a/src/hotspot/cpu/riscv/macroAssembler_riscv.hpp b/src/hotspot/cpu/riscv/macroAssembler_riscv.hpp
index 792c1fc210..65f9153266 100644
--- a/src/hotspot/cpu/riscv/macroAssembler_riscv.hpp
+++ b/src/hotspot/cpu/riscv/macroAssembler_riscv.hpp
@@ -540,8 +540,6 @@ class MacroAssembler: public Assembler {
   }

   // mv
-  void mv(Register Rd, address addr)                    { li(Rd, (int64_t)addr);  }
-
   inline void mv(Register Rd, int imm64)                { li(Rd, (int64_t)imm64); }
   inline void mv(Register Rd, long imm64)               { li(Rd, (int64_t)imm64); }
   inline void mv(Register Rd, long long imm64)          { li(Rd, (int64_t)imm64); }
@@ -552,6 +550,7 @@ class MacroAssembler: public Assembler {
   inline void mvw(Register Rd, int32_t imm32) { mv(Rd, imm32); }

   void mv(Register Rd, Address dest);
+  void mv(Register Rd, address dest);
   void mv(Register Rd, RegisterOrConstant src);

   // logic


`stop()` is the right position; however, you are right: there is another small issue inside.
The reason is inside [the reversion for JDK-8248404](https://github.com/openjdk/riscv-port-jdk11u/pull/3/commits/d8b14fd5e6455b47cfcb02d13c0c24c74e824570#diff-7a5c3ed05b6f3f06ed1c59f5fc2a14ec566a6a5bd1d09606115767daa99115bdL1367-L1372), where I removed the fixed-length `mv` when comparing to the latest TIP jdk code. It was not needed eventually so I removed it by the way - but sadly we still need it for the initial load since `mv()` still redirects to `li()` after the change. My apologies for this issue.

I no longer have access to a RISC-V environment - so @kuaiwei could you please have a look at this fix when available? Thank you.

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

PR Comment: https://git.openjdk.org/riscv-port-jdk11u/pull/3#issuecomment-1729802313