On Thu, 31 Oct 2024 16:27:05 GMT, Patricio Chilano Mateo <pchilanom...@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? I also >> think we should fix the aarch64 c2 stub to just store last_Java_pc like you >> suggest. Adjusting the stack like this has in the past caused other >> problems, in particular making it hard to obtain safe stack traces during >> asynchronous profiling. >> >> It's still unclear to me exactly how we resume after preemption. It looks >> like we resume at last_Java_pc with rsp set based on last_Java_sp, which is >> why it needs to be adjusted. If that's the case, an alternative >> simplification for aarch64 is to set a different last_Java_pc that is >> preemption-friendly that skips the stack adjustment. In your example, >> last_Java_pc would be set to 0xffffdfdba5e4. I think it is a reasonable >> requirement that preemption can return to last_Java_pc/last_Java_sp without >> adjustments. > >> 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1825498409
