RFR: 8327647: Occasional SIGSEGV in markWord::displaced_mark_helper() for SPECjvm2008 sunflow

[Matias Saavedra Silva]

On Tue, 26 Mar 2024 12:29:26 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 3085:
>> 
>>> 3083:   __ push(r0);
>>> 3084:   // R1: field offset, R2: TOS, R3: flags
>>> 3085:   load_resolved_field_entry(r2, r2, r0, r1, r3);
>> 
>> It is useless to use r0 here, so can we change it to noreg and eliminate the use of push(r0)/pop(r0)?
>
> I also noticed this today while looking at the code and I was testing the following change:
> [unnecessary-tos-load-v2.diff.txt](https://github.com/openjdk/jdk/files/14758318/unnecessary-tos-load-v2.diff.txt)
> 
> @matias9927 : Can you this extra change while you are on it? I think we should fix this for both performance and correctness reasons. The 8-byte push/pop would violate the AArch64 & RISC-V ABI which specifies that alignment of  SP should always be 16 bytes [1][2].
> 
> [1] https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/using-the-stack-in-aarch32-and-aarch64
> [2] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc

Noted! I haven't been able to replicate the crash yet, but I think it will be worth moving forward with this patch as a matter of correctness.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18477#discussion_r1559947691