On Tue, 13 Jul 2021 13:52:18 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
>> This patch rewrites the prologue and epilogue of panama upcalls, in order to
>> fix the test failure from the title.
>>
>> Previously, we did a call to potentially attach the current thread to the
>> VM, and then afterwards did the same suspend and stack reguard checks that
>> we do on the back-edge of a native downcall. Then, on the back edge of the
>> upcall we did another conditional call to detach the thread.
>>
>> The suspend and reguard checks on the front-edge are incorrect, so I've
>> changed the 2 calls to mimic what is done by JavaCallWrapper instead (with
>> attach and detach included), and removed the old suspend and stack reguard
>> checks.
>>
>> FWIW, this removes the JavaFrameAnchor save/restore MacroAssembler code.
>> This is now written in C++. Also, MacroAssembler code was added to
>> save/restore the result of the upcall around the call on the back-edge,
>> which was previously missing. Since the new code allocates a handle block as
>> well, I've added handling for those oops to frame & OptimizedUpcallBlob.
>>
>> Testing: local running of `jdk_foreign` on Windows and Linux (WSL). Tier 1-3
>
> src/hotspot/cpu/x86/frame_x86.cpp line 356:
>
>> 354: }
>> 355:
>> 356: OptimizedEntryBlob::FrameData*
>> OptimizedEntryBlob::frame_data_for_frame(const frame& frame) const {
>
> It makes sense to assert that `frame` refers to optimized entry frame
> (`assert(frame->is_optimized_entry_frame(), "wrong frame");`.
Good idea. Thanks
> src/hotspot/share/runtime/thread.hpp line 1128:
>
>> 1126:
>> 1127: private:
>> 1128: DEBUG_ONLY(void verify_frame_info();)
>
> If you declare `verify_frame_info` as returning a `bool` (and just put a
> `return true;` at the end of `JavaThread::verify_frame_info()`), you can call
> it as:
>
> assert(verify_frame_info(), "unexpected frame info");
Thanks for the suggestion. I'd like to keep it the way it is though, so that
the assert message contains the `has_last_frame` & `java_call_counter` values.
(this was one of the reason I did this refactor as well, since the assert I was
hitting out of those 3 didn't contain that info).
-------------
PR: https://git.openjdk.java.net/jdk17/pull/149
