RFR: 8248817: Windows: Improving common cross-platform code

Ludovic Henry luhenry at microsoft.com
Thu Jul 16 22:00:37 UTC 2020


I've upload these latest changes to http://cr.openjdk.java.net/~burban/luhenry/8248817-atomics/webrev.04

________________________________________
From: Ludovic Henry <luhenry at microsoft.com>
Sent: Thursday, July 16, 2020 11:27
To: Kim Barrett
Cc: David Holmes; hotspot-runtime-dev at openjdk.java.net; openjdk-aarch64
Subject: Re: RFR: 8248817: Windows: Improving common cross-platform code

Hi Kim,

> atomic_windows_x86.hpp:
> I think retaining the "stub" nomenclature is misleading; "stub" has a
> somewhat specific meaning in low-level HotSpot code.  "intrinsic"
> might be a better choice.

Ok, let me rename that to IntrinsicName and IntrinsicType.

> atomic_windows_x86.hpp:
> I'm guessing that as a followup, as part of the aarch64 port, these
> will probably be changed to dispatch on the memory order to choose the
> appropriately ordered intrinsics.  But what's being proposed seems
> sufficient for now.

This code will not be shared directly with aarch64, even though it is very similar. As part of landing the Windows-AArch64 changes, we can imagine merging the two into src/hotspot/os/windows/atomic_windows.hpp for example.

> atomic_windows_x86.hpp:
> I noticed that HotSpot's cmpxchg argument order is different than the
> MSVC intrinsic's argument order.  That might be worthy of a comment.

Good point, let me add a comment.

> That parameter alignment problem doesn’t seem to have been fixed.

Bernhard (whom I'm using his CR) hasn't had a chance to upload it yet, I'll let you know once it's done.

> Nice to see the stub functions go away.

It does indeed seems to simplify the code drastically, so happy to participate to that as well :)

Thank you,

________________________________________
From: Kim Barrett <kim.barrett at oracle.com>
Sent: Thursday, July 16, 2020 11:20
To: Ludovic Henry
Cc: David Holmes; hotspot-runtime-dev at openjdk.java.net; openjdk-aarch64
Subject: Re: RFR: 8248817: Windows: Improving common cross-platform code

> On Jul 16, 2020, at 11:39 AM, Ludovic Henry <luhenry at microsoft.com> wrote:
>
> Hi David,
>
>> I'm not very good with templates so I've asked Kim Barrett if he can
>> take a look at this aspect.
>
> Sounds good, looking forward for his review.

atomic_windows_x86.hpp:
I think retaining the "stub" nomenclature is misleading; "stub" has a
somewhat specific meaning in low-level HotSpot code.  "intrinsic"
might be a better choice.

atomic_windows_x86.hpp:
I'm guessing that as a followup, as part of the aarch64 port, these
will probably be changed to dispatch on the memory order to choose the
appropriately ordered intrinsics.  But what's being proposed seems
sufficient for now.

atomic_windows_x86.hpp:
I noticed that HotSpot's cmpxchg argument order is different than the
MSVC intrinsic's argument order.  That might be worthy of a comment.

> I'm updating the webrev at https://nam06.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~burban%2Fluhenry%2F8248817-atomics%2Fwebrev.03&data=02%7C01%7Cluhenry%40microsoft.com%7C06667a1f27ef4b6bc47208d829b5d4c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637305208221370933&sdata=c278Ey9Aoc%2BOARBRc1SRxIgdBt9N5d3rf9jlsgqFamg%3D&reserved=0 with the proper realignement of the parameters.

That parameter alignment problem doesn’t seem to have been fixed.

Other than these minor nits, this change looks good to me.

Nice to see the stub functions go away.



More information about the hotspot-runtime-dev mailing list