On Tue, 2 Feb 2021 21:49:36 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> support macos_aarch64 in hsdis
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 323:
>
>> 321: str(zr, Address(rthread, JavaThread::last_Java_pc_offset()));
>> 322:
>> 323: str(zr, Address(rthread, JavaFrameAnchor::saved_fp_address_offset()));
>
> I don't think this switch from `JavaThread::saved_fp_address_offset()`
> to `JavaFrameAnchor::saved_fp_address_offset()` is correct since
> `rthread` is still used and is a JavaThread*. The new code will give you:
>
> `rthread` + offset of the `saved_fp_address` field in a JavaFrameAnchor
>
> The old code gave you:
>
> `rthread` + offset of the `saved_fp_address` field in the JavaFrameAnchor
> field in the JavaThread
>
> Those are not the same things.
I agree, I don't understand why this change was made.
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line 45:
>
>> 43: // Atomically copy 64 bits of data
>> 44: static void atomic_copy64(const volatile void *src, volatile void
>> *dst) {
>> 45: *(jlong *) dst = *(const jlong *) src;
>
> Is this construct actually atomic on aarch64?
Yes.
> src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.hpp line 37:
>
>> 35:
>> 36: private:
>> 37:
>
> 'private' usually has one space in front of it.
> Also, why the blank line after it?
It reads better with the blank line, and it's not in violation of HotSpot
conventions.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2200
