On Thu, 16 Oct 2025 16:23:05 GMT, Patricio Chilano Mateo <[email protected]> wrote:
>> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 209: >> >>> 207: // the last_sp saved in the frame (remove possible alignment added >>> while >>> 208: // thawing, see ThawBase::finish_thaw()). We also need to clear >>> the last_sp >>> 209: // saved in the frame as it is not expected to be set in case we >>> preempt again. >> >> A bit stronger? >> Suggestion: >> >> // saved in the frame because it must be clear if we freeze again. > > Just to add more context, not clearing last_sp will make this assert [1] fire > if we freeze again. That assert is mostly a verification check, because we > know the interpreter doesn’t set last_sp for the top frame when calling into > the VM. But I don’t see a fundamental reason why it must be cleared (removing > the assert and not clearing last_sp works). I don’t see any other code that > checks last_sp needs to be cleared for the top frame (other than in the > interpreter before calling into the VM). > How about changing that last sentence with: `We also clear last_sp to match > the behavior when calling the VM from the interpreter (we check for this in > FreezeBase::prepare_freeze_interpreted_top_frame).` > > [1] > https://github.com/openjdk/jdk/blob/87092ef1d97e00ddb6674b0e309f2f904d307604/src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp#L136 FWIW, interpreter_frame_tos_address() behaves differently depending on if last_sp() is cleared or not. I know deoptimization sets last_sp temporarily but makes sure to clear it before giving control back to the interpreter. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2462366413
