Re: RFR: 8318776: Require supports_cx8 to always be true [v7]

2023-11-23 Thread David Holmes
On Thu, 23 Nov 2023 12:09:27 GMT, David Holmes  wrote:

>> As discussed in JBS all platforms (some tweaks to Zero are in progress) 
>> actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip 
>> out the locked-based alternatives to using it and just add a guarantee that 
>> it is true at runtime. And all platforms except some ARM variants set 
>> `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes:
>> - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not 
>> defined
>> - Assertions for `supports_cx8()` are removed
>> - Compiler predicates requiring `supports_cx8()` are removed
>> - Access backend is greatly simplified without the need for lock-based 
>> alternative
>> - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the 
>> need for a lock-based alternative
>> 
>> I did consider moving all the ARM `kuser_helper` related code to be only 
>> defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a 
>> theoretical risk this could change the behaviour if ARMv7 binaries were run 
>> on other ARM CPU's. I added a note to that effect in 
>> vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up 
>> further if desired.
>> 
>> Testing:
>> - All Oracle tiers 1-5 builds (which includes an ARMv7 build)
>> - GHA builds/tests
>> - Oracle tiers 1-3 sanity testing
>> 
>> Zero changes coming in via 
>> [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged 
>> when they arrive.
>> 
>> Thanks.
>
> David Holmes has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge
>  - Fix typo
>  - Merge with master and update Zero code accordingly
>  - Merge branch 'master' into 8318776-supports_cx8
>  - Remove unnecessary includes of vm_version.hpp.
>Fix copyright years.
>  - Remove cx8 comment as no longer relevant (the spinlock is used regardless 
> of cx8)
>  - Remove suports_cx8() checks from gtest
>  - Remove test for VMSupportsCX8
>  - 8318776: Require supports_cx8 to always be true

Thanks for all the reviews. Integrating now.

-

PR Comment: https://git.openjdk.org/jdk/pull/16625#issuecomment-1824966904


Re: RFR: 8318776: Require supports_cx8 to always be true [v7]

2023-11-23 Thread David Holmes
> As discussed in JBS all platforms (some tweaks to Zero are in progress) 
> actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip 
> out the locked-based alternatives to using it and just add a guarantee that 
> it is true at runtime. And all platforms except some ARM variants set 
> `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes:
> - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not 
> defined
> - Assertions for `supports_cx8()` are removed
> - Compiler predicates requiring `supports_cx8()` are removed
> - Access backend is greatly simplified without the need for lock-based 
> alternative
> - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the 
> need for a lock-based alternative
> 
> I did consider moving all the ARM `kuser_helper` related code to be only 
> defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a 
> theoretical risk this could change the behaviour if ARMv7 binaries were run 
> on other ARM CPU's. I added a note to that effect in 
> vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up 
> further if desired.
> 
> Testing:
> - All Oracle tiers 1-5 builds (which includes an ARMv7 build)
> - GHA builds/tests
> - Oracle tiers 1-3 sanity testing
> 
> Zero changes coming in via 
> [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged 
> when they arrive.
> 
> Thanks.

David Holmes has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains ten commits:

 - Merge
 - Fix typo
 - Merge with master and update Zero code accordingly
 - Merge branch 'master' into 8318776-supports_cx8
 - Remove unnecessary includes of vm_version.hpp.
   Fix copyright years.
 - Remove cx8 comment as no longer relevant (the spinlock is used regardless of 
cx8)
 - Remove suports_cx8() checks from gtest
 - Remove test for VMSupportsCX8
 - 8318776: Require supports_cx8 to always be true

-

Changes: https://git.openjdk.org/jdk/pull/16625/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=16625=06
  Stats: 460 lines in 39 files changed: 16 ins; 429 del; 15 mod
  Patch: https://git.openjdk.org/jdk/pull/16625.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16625/head:pull/16625

PR: https://git.openjdk.org/jdk/pull/16625