RFR: 8316961: Fallback implementations for 64-bit Atomic::{add,xchg} on 32-bit platforms

[Aleksey Shipilev]

On Thu, 19 Oct 2023 13:32:49 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> EDIT: Wait, something is off here. I am running on RPi 4 that is definitely ARM v7+, which should support this. I think the gtest does not initialize `VM_Version` or something...

Oh, I see, that's the test bug: #16269 

The contract for `Atomic` requires that `PlatformCmpxchg<8>` is implemented on all platforms:


  // Platform-specific implementation of cmpxchg.  Support for sizes
  // of 1, 4, and 8 are required.  ...
 ...
  template<size_t byte_size> struct PlatformCmpxchg;


ARM32 does not implement this for all types of machines, as per comment here:


/*
 * Atomic long operations on 32-bit ARM
 * ARM v7 supports LDREXD/STREXD synchronization instructions so no problem.
 * ARM < v7 does not have explicit 64 atomic load/store capability.
 * However, gcc emits LDRD/STRD instructions on v5te and LDM/STM on v5t
 * when loading/storing 64 bits.
 * For non-MP machines (which is all we support for ARM < v7)
 * under current Linux distros these instructions appear atomic.
 * See section A3.5.3 of ARM Architecture Reference Manual for ARM v7.
 * Also, for cmpxchg64, if ARM < v7 we check for cmpxchg64 support in the
 * Linux kernel using _kuser_helper_version. See entry-armv.S in the Linux
 * kernel source or kernel_user_helpers.txt in Linux Doc.
 */


I guess we still have a problem for ARM < v7 without kernel support, which would either assert in `atomic_linux_arm.hpp`, or `stop` later in the stub. I would argue that is a violation of `Atomic` contract that requires implementing `PlatformCmpxchg<8>` one way or the other. 

Given how even current direct uses of `PlatformCmpxchg<8>` in those configs would fail, I don't see this PR introduces regressions: using `PlatformXchg<8>` and `PlatformAdd<8>` would break on those platform either before or after this change, and thus we can proceed even with this change. (I would pull in `TEST_VM` change once committed.) 

Agree, @dholmes-ora?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16252#issuecomment-1771245837