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

Dean Long dlong at openjdk.org
Wed Sep 21 22:37:16 UTC 2022


On Wed, 21 Sep 2022 07:04:08 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> The method `frame::interpreter_frame_last_sp()` is a platform method in the sense that it is not declared in a shared header file. It is declared and defined on some platforms though (x86 and aarch64 I think).
>> 
>> `frame::interpreter_frame_last_sp()` existed on these platforms before vm continuations (aka loom). Shared code that is part of the vm continuations implementation references it. This breaks the platform abstraction.
>> 
>> Using unextended_sp is problematic too because there are no guarantees by the platform abstraction layer for it. In fact unextended_sp < sp is possible on ppc64 and aarch64.
>> 
>> This fix changes the callers of is_sp_in_continuation()
>> 
>> ```c++
>> static inline bool is_sp_in_continuation(const ContinuationEntry* entry, intptr_t* const sp) {
>>   return entry->entry_sp() > sp;
>> }
>> 
>> 
>> to pass the actual sp. This is correct because the following is true on all platforms:
>> 
>> ```c++
>> a.sp() > E->entry_sp() > b.sp() > c.sp()
>> 
>> 
>> where `a`, `b`, `c` are stack frames in call order and `E` is a ContinuationEntry. `a` is the caller frame of the continuation entry frame that corresponds to `E`.
>> 
>> is_sp_in_continuation() will then return true for `b.sp()` and `c.sp()` and false for `a.sp()`
>> 
>> Testing: hotspot_loom and jdk_loom on x86_64 and aarch64.
>
> Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove `Unimplemented` definitions of interpreter_frame_last_sp

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.  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().

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

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


More information about the hotspot-compiler-dev mailing list