RFR: 8318157: RISC-V: implement ensureMaterializedForStackWalk intrinsic

Olga Mikhaltsova omikhaltcova at openjdk.org
Wed Nov 29 22:12:04 UTC 2023


On Wed, 29 Nov 2023 02:08:10 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hi, thanks for finding this. Looks OK. Any correctness test performed?
>
>> @RealFYang, yes, tier1 tests passed.
>> 
>> The only thing, I'm hesitating about, is the size of __ nop(). I assume that it'll be 4 bytes because, for example, I see in code related to deoptimization the following: [src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp#L1290](https://github.com/openjdk/jdk/blob/a40d8d97e84d88d1a65aba81bfc09339be95e427/src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp#L1290)
>> 
>> ```
>>     // First instruction must be a nop as it may need to be patched on deoptimisation
>>     {
>>       Assembler::IncompressibleRegion ir(masm);  // keep the nop as 4 bytes for patching.
>>       MacroAssembler::assert_alignment(__ pc());
>>       __ nop();  // 4 bytes
>>     } 
>> ```
>> 
>> But from the other hand, according to the comments, in case of RVC __ nop() shall be 2 bytes: [src/hotspot/cpu/riscv/riscv.ad#L1292](https://github.com/openjdk/jdk/blob/98f7d263915e20a78f1ab53016079e3f15988aa3/src/hotspot/cpu/riscv/riscv.ad#L1292)
>> 
>> ```
>>     Assembler::CompressibleRegion cr(&_masm); // nops shall be 2-byte under RVC for alignment purposes.
>>     for (int i = 0; i < _count; i++) {
>>       __ nop();
>>     }
>> ```
>> 
>> Is there possibility of error?
> 
> Note that there is a `Assembler::IncompressibleRegion ir(&_masm);` declaration on the function entry [1]. This effectivly ensures that the `nop` planted here won't be compressed and thus will be 4-bytes in size. 
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/riscv.ad#L2369

@RealFYang thanks a lot for the explanation and reviewing!

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

PR Comment: https://git.openjdk.org/jdk/pull/16808#issuecomment-1832780192


More information about the hotspot-compiler-dev mailing list