On Fri, 1 Nov 2024 07:14:35 GMT, Dean Long <dl...@openjdk.org> wrote:
>>> OK, so you're saying it's the stack adjustment that's the problem. It
>>> sounds like there is code that is using rsp instead of last_Java_sp to
>>> compute the frame boundary. Isn't that a bug that should be fixed?
>>>
>> It's not a bug, it's just that the code from the runtime stub only cares
>> about the actual rsp, not last_Java_sp. We are returning to the pc right
>> after the call so we need to adjust rsp to what the runtime stub expects.
>> Both alternatives will work, either changing the runtime stub to set last pc
>> and not push those two extra words, or your suggestion of just setting the
>> last pc to the instruction after the adjustment. Either way it requires to
>> change the c2 code though which I'm not familiar with. But if you can
>> provide a patch I'm happy to apply it and we can remove this
>> `possibly_adjust_frame()` method.
>
> It turns out if we try to set last pc to the instruction after the
> adjustment, then we need an oopmap there, and that would require more C2
> changes. Then I thought about restoring SP from FP or last_Java_fp, but I
> don't think we can rely on either of those being valid after resume from
> preemption, so I'll try the other alternative.
Here's my suggested C2 change:
diff --git a/src/hotspot/cpu/aarch64/aarch64.ad
b/src/hotspot/cpu/aarch64/aarch64.ad
index d9c77a2f529..1e99db191ae 100644
--- a/src/hotspot/cpu/aarch64/aarch64.ad
+++ b/src/hotspot/cpu/aarch64/aarch64.ad
@@ -3692,14 +3692,13 @@ encode %{
__ post_call_nop();
} else {
Label retaddr;
+ // Make the anchor frame walkable
__ adr(rscratch2, retaddr);
+ __ str(rscratch2, Address(rthread, JavaThread::last_Java_pc_offset()));
__ lea(rscratch1, RuntimeAddress(entry));
- // Leave a breadcrumb for JavaFrameAnchor::capture_last_Java_pc()
- __ stp(zr, rscratch2, Address(__ pre(sp, -2 * wordSize)));
__ blr(rscratch1);
__ bind(retaddr);
__ post_call_nop();
- __ add(sp, sp, 2 * wordSize);
}
if (Compile::current()->max_vector_size() > 0) {
__ reinitialize_ptrue();
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1826252551
