RFR: 8234563: Harmonize parameter order in Atomic

[Robbin Ehn]

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