On Wed, 24 Aug 2022 11:33:01 GMT, Fei Yang <fy...@openjdk.org> wrote:
>>> Do you have details about testing performed in Native Image as mentioned in >>> PR decription? >> >> Yes, the RISC-V LLVM backend for Native Image passes 99% of the tests >> performed, which is similar to the other LLVM backends. >> >>> I see you added more changes in hotspot file sharedRuntime_riscv.cpp >>> guarded by macro INCLUDE_JVMCI. Searching for INCLUDE_JVMCI or >>> COMPILER2_OR_JVMCI in src/hotspot/cpu/aarch64, I see several more places >>> checking for these macros. Have you checked if we need similar changes for >>> your use case? >> >> I first added the changes for all places where those macros are used, but >> since only modifying sharedRuntime_riscv.cpp was enough to make the tests >> pass, I did not wanted to add code that I was not sure was useful at the >> moment. >> >>> Also could you explain the change made in hotspot file deoptimization.hpp? >>> Thanks. >> >> When the method is inlined, the `if (trap_request < 0)` check behaves >> incorrectly when the `nmethod` is compiled by JVMCI. Even though the boolean >> is true, the function returns -11 instead of -1, and the `if >> (unloaded_class_index >= 0)` checks have the same issue, causing an access >> to an illegal index of an array. I am not sure why this happens, as it works >> correctly for method not compiled by JVMCI. > >> > I see you added more changes in hotspot file sharedRuntime_riscv.cpp >> > guarded by macro INCLUDE_JVMCI. Searching for INCLUDE_JVMCI or >> > COMPILER2_OR_JVMCI in src/hotspot/cpu/aarch64, I see several more places >> > checking for these macros. Have you checked if we need similar changes for >> > your use case? >> >> I first added the changes for all places where those macros are used, but >> since only modifying sharedRuntime_riscv.cpp was enough to make the tests >> pass, I did not wanted to add code that I was not sure was useful at the >> moment. > > Well, that sounds fragile to me since you are depending on a relatively small > set of JTreg tests here. I think an analysis is needed here to be sure about > whether those are really needed or not. @RealFYang are you ok with deferring further changes to a future RFE? ------------- PR: https://git.openjdk.org/jdk/pull/9587
