RFR: 8327647: Occasional SIGSEGV in markWord::displaced_mark_helper() for SPECjvm2008 sunflow
Fei Yang
fyang at openjdk.org
Thu Apr 11 21:19:50 UTC 2024
On Wed, 10 Apr 2024 19:16:42 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:
>> 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.
Note that `noreg` is currently not properly handled / considered in `load_resolved_field_entry`. And it doesn't look nice to me to only check if `tos_state` with `noreg` in this function. That's why I replaced call to `load_resolved_field_entry` with seperate loads in my propsed fix.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18477#discussion_r1560311323
More information about the hotspot-compiler-dev
mailing list