RFR: 8290965: PPC64: Implement post-call NOPs [v2]

Richard Reingruber rrich at openjdk.org
Wed Jan 10 10:38:25 UTC 2024


On Wed, 27 Dec 2023 18:27:22 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix comment
>>   
>>   Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>
> src/hotspot/cpu/ppc/frame_ppc.hpp line 414:
> 
>> 412:   // Constructors
>> 413:   inline frame(intptr_t* sp, intptr_t* fp, address pc);
>> 414:   inline frame(intptr_t* sp, address pc, kind knd = kind::nmethod);
> 
> I think using `kind::nmethod` by default is potentially dangerous. The pc may be outside of the code cache and calling find_blob_fast would be unreliable. It's used by pns for debugging code. It doesn't look performance critical and we could use a conservative default.
> I guess that we don't see issues because native code doesn't set bit 9 in CMPI/CMPLI.

`pns` does not use this constructor. It uses `frame::frame(void* sp, void* fp, void* pc) : frame((intptr_t*)sp, (address)pc, kind::code_blob)`. So there's no problem. `pns` seems to be the only user of this one. It might good to use `kind::native` there.

Using `kind::native` (or `kind::unknow`) as default instead of `kind::nmethod` is potentially problematic since there might be locations in shared code that should set `kind::nmethod`. I think this requires a clean-up of the shared frame api. Note also that using the wrong kind (wrong constructor on other platfroms) hit the assertion in `CodeCache::find_blob_and_oopmap` (that's how I noticed that the distinction is actually needed :))

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17171#discussion_r1447187050


More information about the hotspot-compiler-dev mailing list