RFR: 8322294: Cleanup NativePostCallNop

[Martin Doerr]

On Mon, 18 Dec 2023 22:05:32 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

> This is a refactoring/cleanup of `NativePostCallNop` that simplifies the ppc64 port (dependent pr https://github.com/openjdk/jdk/pull/17171).
> 
> * `frame::get_oop_map()` is moved to shared code
> 
> * encoding / decoding details of the oopmap slot and the CodeBlob offset are moved from shared code to the platform dependent implementations of `bool NativePostCallNop::patch(int32_t oopmap_slot, int32_t cb_offset)` and `bool NativePostCallNop::decode(int32_t& oopmap_slot, int32_t& cb_offset)`
> 
> The change passed our CI testing. JTReg tests: tier1-4 of hotspot and jdk. All of Langtools and jaxp. SPECjvm2008, SPECjbb2015, Renaissance Suite, and SAP specific tests.
> All testing was done with fastdebug and release builds on the main platforms and also on Linux/PPC64le and AIX.

Nice refactoring! I couldn't spot any bug. Only minor suggestions.

src/hotspot/cpu/s390/nativeInst_s390.hpp line 661:

> 659:   bool check() const { Unimplemented(); return false; }
> 660:   bool decode(int32_t& oopmap_slot, int32_t& cb_offset) const { return false; }
> 661:   bool patch(int32_t oopmap_slot, int32_t cb_offset) { Unimplemented() ; return false; }

Whitespace between `()` and `;`.

src/hotspot/share/runtime/frame.inline.hpp line 109:

> 107: inline const ImmutableOopMap* frame::get_oop_map() const {
> 108:   if (_cb == nullptr) return nullptr;
> 109:   if (_cb->oop_maps() != nullptr) {

Could be shorter: `if (_cb == nullptr || _cb->oop_maps() == nullptr) return nullptr;`

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

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17150#pullrequestreview-1794338004
PR Review Comment: https://git.openjdk.org/jdk/pull/17150#discussion_r1434966035
PR Review Comment: https://git.openjdk.org/jdk/pull/17150#discussion_r1434968074