RFR: 8234563: Harmonize parameter order in Atomic

[Stefan Karlsson]

Thanks, Robbin.

StefanK

On 2019-11-22 10:29, Robbin Ehn wrote:
> Thanks again, looks good!
> 
> Much appreciated!
> 
> /Robbin
> 
> On 11/21/19 12:26 PM, Stefan Karlsson wrote:
>> Hi all,
>>
>> I'd like to propose a restructuring of the parameter order in Atomic.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8234563
>>
>> These are some of the functions used to concurrently write memory from 
>> HotSpot C++ code:
>>
>>   Atomic::store(value, destination);
>>   OrderAccess::release_store(destination, value)
>>   OrderAccess::release_store_fence(destination, value)
>>   Atomic::add(value, destination);
>>   Atomic::sub(value, destination);
>>   Atomic::xchg(exchange_value, destination);
>>   Atomic::cmpxchg(exchange_value, destination, compare_value);
>>
>> With the proposed JDK-8234562 change, this would look like:
>>
>>   Atomic::store(value, destination);
>>   Atomic::release_store(destination, value)
>>   Atomic::release_store_fence(destination, value)
>>   Atomic::add(value, destination);
>>   Atomic::sub(value, destination);
>>   Atomic::xchg(exchange_value, destination);
>>   Atomic::cmpxchg(exchange_value, destination, compare_value);
>>
>> I'd like to propose that we move the destination parameter over to the 
>> left, and the new value to the right. This would look like this:
>>
>>   Atomic::store(destination, value);
>>   Atomic::release_store(destination, value)
>>   Atomic::release_store_fence(destination, value)
>>   Atomic::add(destination, value);
>>   Atomic::sub(destination, value);
>>   Atomic::xchg(destination, exchange_value);
>>   Atomic::cmpxchg(destination, compare_value, exchange_value);
>>
>> This would bring the Atomic API more in-line with the order for a 
>> normal store:
>>
>> *destination = value;
>>
>> I've split this up into separate patches, each dealing with a separate 
>> operation:
>>
>> Atomic::store:
>>   https://cr.openjdk.java.net/~stefank/8234563/webrev.01.01.store/
>>
>> Atomic::add:
>>   https://cr.openjdk.java.net/~stefank/8234563/webrev.01.02.add/
>>
>> Atomic::sub:
>>   https://cr.openjdk.java.net/~stefank/8234563/webrev.01.03.sub/
>>
>> Atomic::xchg:
>>   https://cr.openjdk.java.net/~stefank/8234563/webrev.01.04.xchg/
>>
>> Atomic::cmpxchg:
>>   https://cr.openjdk.java.net/~stefank/8234563/webrev.01.05.cmpxchg/
>>
>> All sub-patches combined:
>>   https://cr.openjdk.java.net/~stefank/8234563/webrev.01.all/
>>
>> The patches applies on-top of the patch for JDK-8234562:
>>   https://cr.openjdk.java.net/~stefank/8234562/webrev.01/
>>
>> I've tested this patch with tier1-7. I've also built fastdebug on the 
>> following configs: aarch64, arm32, ppc64le, s390x, shenandoah, zero, 
>> minimal, just to minimize any disruptions. However, this is a moving 
>> target and latest rebase of this has only been run on tier1.
>>
>> Thanks,
>> StefanK