RFR: 8293117: Add atomic bitset functions [v2]

[David Holmes]

On Sat, 29 Apr 2023 01:10:23 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change that adds and/or/xor bitops to Atomic.
>> 
>> Some offline discussion reached consensus on the following API (with T being a
>> template parameter that must be an integral type):
>>   T fetch_then_and(volatile T* dest, T bits, atomic_memory_order order)
>>   T fetch_then_or(volatile T* dest, T bits, atomic_memory_order order)
>>   T fetch_then_xor(volatile T* dest, T bits, atomic_memory_order order)
>>   T and_then_fetch(volatile T* dest, T bits, atomic_memory_order order)
>>   T or_then_fetch(volatile T* dest, T bits, atomic_memory_order order)
>>   T xor_then_fetch(volatile T* dest, T bits, atomic_memory_order order)
>> with order defaulting to memory_order_conservative.
>> 
>> I'm hoping there won't be more bike-shedding on the names.
>> 
>> This naming convention differs from what exists for add/sub, where we have
>> fetch_and_add, add_and_fetch, and the like.  fetch_and_and and and_and_fetch
>> just looked too weird.  For consistency we should probably rename the add/sub
>> operations to use "then".
>> 
>> A default implementation is provided, using a CAS loop for all operations.
>> That implementation can be overridden on a per-platform basis.  Currently
>> there aren't any platform-specific implementations; such will be added by
>> followup RFEs.  We'll want to override for most (if not all) platforms, since
>> the CAS loop implementation is generally less than optimal (in some cases
>> rather comically so).
>> 
>> For some platforms a CAS loop is necessary to obtain either the old or the new
>> value, but have a better implementation available if neither value is needed.
>> However, we can't easily make use of such.  We would need to either add
>> functions returning void to the API or change the API to return expression
>> templates so we can detect how the result is used.  However, we may be able to
>> use compiler intrinsics for some platforms, which may be able to specialize
>> for the usage context.  If that turns out to be a problem then we can expand
>> the API accordingly later.
>> 
>> Testing:
>> mach5 tier1, including new gtests for the new functionality.
>
> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
> 
>   only run 64bit tests on 64bit platforms

Seems quite reasonable.

I think for the naming it wouldn't be a terrible idea to consider using uppercase for the operation name: AND/OR/XOR/ADD/SUB. That would make the operation name stand out more clearly. Just a thought.

Thanks.

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

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13711#pullrequestreview-1408446112