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