On Sat, 31 Aug 2024 06:52:56 GMT, Fei Yang <fy...@openjdk.org> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Add testcase >> - Remove unnecessary comment from UpcallStub > > src/hotspot/cpu/x86/upcallLinker_x86_64.cpp line 397: > >> 395: * and also should include both saved FP and >> return address >> 396: */ >> 397: (frame_size / wordSize) + 2); > > Hi, I witnessed build failures for other CPU ports. Shouldn't all the > callsites of `UpcallStub::create` be updated to reflect this change? Good catch! I fixed all of caller of `UpcallStub::create`, and then they passed build test. (This PR branch starts with `pr/`, so GHA did not start automatically. Thus I started it in manual.) Test for this PR was kicked in serviceability test, and it works fine on both x86_64 and aarch64. In RISC-V, GHA did not kick the test, but I believe it would work fine because stack structure is similar with x86_64 (return address and FP are seemed to store on the stack) I'm not sure on s390 and PPC64. In PPC64, I checked `UpcallLinker::make_upcall_stub`. It seems to push into the stack once as following, but I'm not sure. address start = __ function_entry(); // called by C __ save_LR_CR(R0); assert((abi._stack_alignment_bytes % 16) == 0, "must be 16 byte aligned"); // allocate frame (frame_size is also aligned, so stack is still aligned) __ push_frame(frame_size, tmp); s390 also pushes address into the stack, but SA does not have s390 implementation, so we might be able to tackle this later when SA supports s390. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20789#discussion_r1739708623
