Re: RFR: 8318776: Require supports_cx8 to always be true [v6]
On Thu, 23 Nov 2023 03:14: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 incrementally with one additional > commit since the last revision: > > Fix typo Ran full jcstress on linux-arm-zero-release on RPi 4 without problem. - Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16625#pullrequestreview-1745896266
Re: RFR: 8318776: Require supports_cx8 to always be true [v6]
On Wed, 22 Nov 2023 21:57:57 GMT, Daniel D. Daugherty wrote: >> My point is that it is such an easy thing to do: leave the "cx8" flag >> sensing code in, and keep setting up `_supports_cx8` based on it for >> `!_LP64` paths. This both provides more safety by failing cleanly on non-CX8 >> platform, and gives other platforms some guidance: if you can check >> something is supported, check it. I think we are generally trying to fail >> cleanly on unsupported configs, if that is easy to achieve. >> >> But now that you nerd-sniped me into this... I think non-CX8 platforms would >> probably predate Pentium. The oldest real machine my lab has is Z530, which >> already has CX8. But it was easy to also go to my QEMU-driven build-test >> server, ask for `i486` as platform there, and et voila, no `cx8` in CPU >> flags: >> >> >> buildworker-debian12-32:~$ lscpu >> Architecture:i486 >> CPU op-mode(s):32-bit >> Address sizes: 36 bits physical, 32 bits virtual >> Byte Order:Little Endian >> CPU(s): 4 >> On-line CPU(s) list: 0-3 >> Vendor ID: GenuineIntel >> Model name:486 DX/4 >> CPU family: 4 >> Model: 8 >> Thread(s) per core: 4 >> Core(s) per socket: 1 >> Socket(s): 1 >> Stepping:0 >> BogoMIPS:5699.99 >> Flags: fpu vme pse apic ht cpuid tsc_known_freq x2apic >> hypervisor cpuid_fault >> >> >> And mainline JDK even starts there! (with interpreter, there are some >> asserts firing in compiler code, having to do with odd instruction selection >> on some paths): >> >> >> $ jdk/bin/java -Xint -version >> openjdk version "22-testing" 2024-03-19 >> OpenJDK Runtime Environment (fastdebug build >> 22-testing-builds.shipilev.net-openjdk-jdk-b627-20231121) >> OpenJDK Server VM (fastdebug build >> 22-testing-builds.shipilev.net-openjdk-jdk-b627-20231121, interpreted mode, >> sharing) > > Nice spelunking... I was wondering if it was something that old. I wasn't > trying to nerd-snipe... > > I was in the dev lab at Intel when Xenix on the i386 first came up and sent > its "Hello World!" email... > I left Intel for Sun in 1987 while i486 was still in development, but I still > had periodic lunches with > folks that worked on those teams. Life was simpler back then... I politely disagree. The whole point here is to leave the past behind as much as possible. We made a concession for ARM32 as there may still be old ARMv5 and ARMv6 systems in use. IIUC you need to go back to the i486 chip to not have cmpxchg8 support and I'd bet money on it that we can't run on such a chip any more for a whole swag of reasons. In any case I don't have an issue telling i486 machine owners they are stuck with JDK 21! - PR Review Comment: https://git.openjdk.org/jdk/pull/16625#discussion_r1402877044
Re: RFR: 8318776: Require supports_cx8 to always be true [v6]
> 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 incrementally with one additional commit since the last revision: Fix typo - Changes: - all: https://git.openjdk.org/jdk/pull/16625/files - new: https://git.openjdk.org/jdk/pull/16625/files/aad0a4c4..2393b9d4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
