RFR: 8289925: Shared code shouldn't reference the platform specific method frame::interpreter_frame_last_sp() [v4]

[Richard Reingruber]

On Wed, 21 Sep 2022 22:35:10 GMT, Dean Long <dlong at openjdk.org> wrote:

> Thinking about this some more, while sp() may work for all platforms, it seems
> like frame::id() is what the shared Loom code should really be using. Then we
> would have:
>
> `a.id() > E->to_frame().id() > b.id() > c.id()`
> 
> The Loom code shouldn't really be assuming that stacks grow in a particular
> direction. That is what frame:is_older() is for.

With that we would have s.th. like

```C++
bool Continuation::is_frame_in_continuation(const ContinuationEntry* entry, const frame& f) {
  return entry->to_frame().is_older(f.id());
}


and replace all uses of is_sp_in_continuation() with it. Not sure if performance
could be a problem then.

```C++
!f.is_older(entry->frame_id());


with a platform dependent definition of frame_id() could be faster then.

> Unfortunately, I found that aarch64 has a problem with this. Given a frame f,
> we can't assert on aarch64 that
> 
> f.sender().is_older(f.id())
> 
> because aarch64 uses unextended_sp() for frame::id().

Yes indeed there seems to be an issue with is_older() on aarch64 because of the
trimming of interpreted frames.

Actually I assumed hotspot does not abstract over stack growth direction because
it did not back in the days when we've ported it to pa-risc where stacks grow
towards higher addresses (yes that's long ago :))

So there might be is_older() now but looking for instance at [class StackOverflow](https://github.com/openjdk/jdk/blob/d5bee4a0dffebcf3037b83fa3f7bc635dd6b1303/src/hotspot/share/runtime/stackOverflow.hpp#L131-L143)
it is clear from the diagrams and the code that stack growth towards lower
addresses is assumed. I'm sure there's still much more shared code in hotspot
with that assumption.

Another example from Loom is the shared code related to JavaThread::_cont_fastpath
where growth towards lower addresses is assumed too.

We could duplicate is_sp_in_continuation() on the platforms or add a static
assertion on a new global definition of the stack growth direction if you
like. Not sure if it is worth it though. I'd tend towards keeping
is_sp_in_continuation() as it is and maybe create an JBS item for stack growth
issues in Loom.

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

PR: https://git.openjdk.org/jdk/pull/9411