On Wed, 24 Aug 2022 11:33:01 GMT, Fei Yang <fy...@openjdk.org> wrote:

> > > 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.

Those parts of code will ultimately be needed for a full JVMCI implementation, 
but adding them right now would leave them untested, since for example, neither 
of the JTreg tests or the RISC-V LLVM backend for Native Image cover the case 
where synchronized code is installed (this corresponds to the code in 
`templateInterpreterGenerator_riscv.cpp`).

However, if it is still better to include those parts of code, I don't mind 
adding the implementation I have for them.

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

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

Reply via email to