On Tue, 3 Nov 2020 13:52:24 GMT, Erik Österlund <eosterl...@openjdk.org> wrote:
>> Erik,
>>
>> Thank you for the update! It looks more elegant.
>>
>> One concern is that after the move of this fragment to the
>> post_method_exit_inner:
>> 1614 if (state == NULL || !state->is_interp_only_mode()) {
>> 1615 // for any thread that actually wants method exit, interp_only_mode
>> is set
>> 1616 return;
>> 1617 }
>> there is no guarantee that the current frame is interpreted below:
>> 1580 if (!exception_exit) {
>> 1581 oop oop_result;
>> 1582 BasicType type =
>> current_frame.interpreter_frame_result(&oop_result, &value);
>> . . .
>> 1597 if (result.not_null() && !mh->is_native()) {
>> 1598 // We have to restore the oop on the stack for interpreter frames
>> 1599 *(oop*)current_frame.interpreter_frame_tos_address() = result();
>> 1600 }
>> Probably, extra checks for current_frame.is_interpreted_frame() in these
>> fragments will be sufficient.
>
>> Erik,
>>
>> Thank you for the update! It looks more elegant.
>>
>> One concern is that after the move of this fragment to the
>> post_method_exit_inner:
>>
>> ```
>> 1614 if (state == NULL || !state->is_interp_only_mode()) {
>> 1615 // for any thread that actually wants method exit, interp_only_mode
>> is set
>> 1616 return;
>> 1617 }
>> ```
>>
>> there is no guarantee that the current frame is interpreted below:
>>
>> ```
>> 1580 if (!exception_exit) {
>> 1581 oop oop_result;
>> 1582 BasicType type =
>> current_frame.interpreter_frame_result(&oop_result, &value);
>> . . .
>> 1597 if (result.not_null() && !mh->is_native()) {
>> 1598 // We have to restore the oop on the stack for interpreter frames
>> 1599 *(oop*)current_frame.interpreter_frame_tos_address() = result();
>> 1600 }
>> ```
>>
>> Probably, extra checks for current_frame.is_interpreted_frame() in these
>> fragments will be sufficient.
>
> That makes sense. Added a check in the latest version that we are in interp
> only mode.
Hi Erik,
I'm not sure, if this fragment is still needed:
1620 if (state == NULL || !state->is_interp_only_mode()) {
1621 // for any thread that actually wants method exit, interp_only_mode is
set
1622 return;
1623 }
Also, can it be that this condition is true:
` (state == NULL || !state->is_interp_only_mode())`
but the top frame is interpreted?
If so, then should we still safe/restore the result oop over a possible
safepoint?
Thanks,
Serguei
-------------
PR: https://git.openjdk.java.net/jdk/pull/930
