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

[Aleksey Shipilev]

On Fri, 20 Oct 2023 10:54:42 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> See the bug for rationale. Looks like there is enough infrastructure to achieve what we want without significant fan-out. I checked all `atomic_*.hpp` headers for unimplemented `PlatformAdd<8>` and `PlatformXchg<8>`, and only these seem to be affected.
>> 
>> Unfortunately, we cannot test these apart from the existing gtest.
>> 
>> Additional testing:
>>  - [x] linux-x86-server-fastdebug, atomic tests pass
>>  - [x] linux-arm-server-fastdebug, atomic tests pass (with #16269 applied)
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Merge branch 'master' into JDK-8316961-fallback-atomics
>  - Work

All right, hear me out. I am trying to briefly recap where we are and where we are going from here.

>From the perspective of caller, the `Atomic` contract is ambiguous. On one hand, it says to check for `supports_cx8()`:


  // Atomic operations on int64 types are not available on all 32-bit
  // platforms. If atomic ops on int64 are defined here they must only
  // be used from code that verifies they are available at runtime and
  // can provide an alternative action if not - see supports_cx8() for
  // a means to test availability.


...and on the other hand, it requires platforms to implement 8-byte CAS:


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


This PR implicitly relies on second assumption. I think the only platform left that needs the `supports_cx8()` is ARM <= 6. The fact that current ARM code does not do consistent support for 64-bit atomics for all platforms is regrettable. To recap, while ARM 7 works fine, the situation with ARM <=6 is more complicated:

Before this PR:
 - `Atomic` unit test would pass
 - Attempt to use 8-byte cmpxchg would fail at runtime (either assert or `stop` in stub)
 - Attempt to use 8-byte add or xchg would fail to link
 
After this PR:
  - `Atomic` unit test would pass (now that it checks for `supports_cx8()`)
  - Attempt to use 8-byte cmpxchg would fail at runtime (either assert or `stop` in stub)
  - Attempt to use 8-byte add or xchg would fail at runtime (either assert or `stop` in stub)
  
So, I don't think this qualifies as regression for ARM <= 6: we are trading the link-time failure for all ARM platforms for clean runtime failure on ARM <= 6 platform, *if* we violate the `supports_cx8()` contract. What we gain with this PR, is that we are able to do 64-bit atomics when `supports_cx8()` is actually true on x86_32 and ARM v7, even if we don't check it.

I dabbled a little in trying to implement 8-byte CAS (locked) implementation for ARM <= 6, and I think it would require some rewrites. There are cases, for example, when we are doing creepy non-atomic stuff when `!os:is_MP` if no hardware support is available. If we go locking route, I think we would also need to check if the atomic load/stores need to be covered by the same lock.

Once/if we fix ARM <= 6, that would eliminate the need for `supports_cx8()` altogether. And I think that is a good thing, because frankly it should not be an `Atomic` caller responsibility to handle obscure platforms in 2023.

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

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