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