RFR (M): 8188224: Generalize Atomic::load/store to use templates

[Erik Österlund]

Hi David,

Thanks for the review.
The comments have been removed.

New full webrev:
http://cr.openjdk.java.net/~eosterlund/8188224/webrev.01/

New incremental webrev:
http://cr.openjdk.java.net/~eosterlund/8188224/webrev.00_01/

Thanks,
/Erik

On 2017-10-03 14:44, David Holmes wrote:
> Hi Erik,
>
> A lot of jumping through hoops just to do a direct load/store in the 
> bulk of cases - but okay, we're embracing templates.
>
> 66   // Atomically store to a location
> 67   // See comment above about using jlong atomics on 32-bit platforms
>
> The comment at #67 and the equivalent one for load can be deleted. The 
> "comment above" should only be referring to r-m-w atomic ops not basic 
> load and store. All platforms must have a means to do atomic 
> load/store of 64-bit due to Java volatile variables (eg by using 
> floating-point unit on 32-bit) but may not have cmpxchg<8> capability. 
> (I failed to convince the author of this when those comments went in. 
> ;-) )
>
> Cheers,
> David
>
> On 3/10/2017 10:29 PM, Erik Österlund wrote:
>> Hi,
>>
>> The time has come to generalize Atomic::load/store with templates - 
>> the last operation to generalize in Atomic.
>> The design was inspired by Atomic::xchg and uses a similar mechanism 
>> to validate the passed in arguments. It was also designed with coming 
>> OrderAccess changes in mind. OrderAccess also contains loads and 
>> stores that will reuse the LoadImpl and StoreImpl infrastructure in 
>> Atomic::load/store. (the type checking for what is okay to pass in to 
>> Atomic::load/store is very much the same for 
>> OrderAccess::load_acquire/*store*).
>>
>> One thing worth mentioning is that the bsd zero port (but notably not 
>> the linux zero port) had a leading fence for atomic stores of jint 
>> when #if !defined(ARM) && !defined(M68K) is true without any comment 
>> describing why. So I took the liberty of removing it. Atomic should 
>> not have any fencing at all - that is what OrderAccess is for. In 
>> fact Atomic does not promise any memory ordering semantics for loads 
>> and stores. Atomic merely provides relaxed accesses that are atomic. 
>> Worth mentioning nevertheless in case anyone wants to keep that jint 
>> Atomic::store fence on bsd zero !M68K && !ARM.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8188224
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8188224/webrev.00/
>>
>> Testing: JPRT, mach5 hs-tier3
>>
>> Thanks,
>> /Erik