Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate compareExchange in favor of compareAndSwap #13934

Merged
merged 4 commits into from
Sep 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 74 additions & 40 deletions modules/internal/Atomics.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -250,30 +250,11 @@ module Atomics {
return ret;
}

/* Equivalent to :proc:`compareExchangeStrong` */
inline proc compareExchange(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
return this.compareExchangeStrong(expected, desired, order);
}

/*
Similar to :proc:`compareExchangeStrong`, except that this function may
return `false` even if the original value was equal to `expected`. This
may happen if the value could not be updated atomically.
*/
inline proc compareExchangeWeak(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
extern externFunc("compare_exchange_weak", bool)
proc atomic_compare_exchange_weak(ref obj:externT(bool), expected:bool, desired:bool, order:memory_order): bool;

var ret:bool;
on this do ret = atomic_compare_exchange_weak(_v, expected, desired, c_memory_order(order));
return ret;
}

/*
Stores `desired` as the new value, if and only if the original value is
equal to `expected`. Returns `true` if `desired` was stored.
*/
inline proc compareExchangeStrong(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
inline proc compareAndSwap(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
extern externFunc("compare_exchange_strong", bool)
proc atomic_compare_exchange_strong(ref obj:externT(bool), expected:bool, desired:bool, order:memory_order): bool;

Expand Down Expand Up @@ -318,6 +299,42 @@ module Atomics {

// Deprecated //

/*
Equivalent to :proc:`compareExchangeStrong`

.. note:: `compareExchange()` is deprecated (and will be repurposed in a
future release), use `compareAndSwap()`.
*/
inline proc compareExchange(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchange is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

/*
Similar to :proc:`compareExchangeStrong`, except that this function may
return `false` even if the original value was equal to `expected`. This
may happen if the value could not be updated atomically.

.. note:: `compareExchangeWeak()` is deprecated (and will be repurposed in a
future release), use `compareAndSwap()`.
*/
inline proc compareExchangeWeak(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

/*
Stores `desired` as the new value, if and only if the original value is
equal to `expected`. Returns `true` if `desired` was stored.

.. note:: `compareExchangeStrong()` is deprecated (and will be repurposed in a
future release), use `compareAndSwap()`.
*/
inline proc compareExchangeStrong(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeStrong is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

pragma "no doc"
inline proc const read(order:memory_order): bool {
compilerWarning("memory_order is deprecated, use memoryOrder");
Expand Down Expand Up @@ -505,30 +522,11 @@ module Atomics {
return ret;
}

/* Equivalent to :proc:`compareExchangeStrong` */
inline proc compareExchange(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
return this.compareExchangeStrong(expected, desired, order);
}

/*
Similar to :proc:`compareExchangeStrong`, except that this function may
return `false` even if the original value was equal to `expected`. This
may happen if the value could not be updated atomically.
*/
inline proc compareExchangeWeak(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
extern externFunc("compare_exchange_weak", T)
proc atomic_compare_exchange_weak(ref obj:externT(T), expected:T, desired:T, order:memory_order): bool;

var ret:bool;
on this do ret = atomic_compare_exchange_weak(_v, expected, desired, c_memory_order(order));
return ret;
}

/*
Stores `desired` as the new value, if and only if the original value is
equal to `expected`. Returns `true` if `desired` was stored.
*/
inline proc compareExchangeStrong(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
inline proc compareAndSwap(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
extern externFunc("compare_exchange_strong", T)
proc atomic_compare_exchange_strong(ref obj:externT(T), expected:T, desired:T, order:memory_order): bool;

Expand Down Expand Up @@ -705,6 +703,42 @@ module Atomics {

// Deprecated //

/*
Equivalent to :proc:`compareExchangeStrong`

.. note:: `compareExchange()` is deprecated (and will be repurposed in a
future release), use `compareAndSwap()`.
*/
inline proc compareExchange(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchange is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

/*
Similar to :proc:`compareExchangeStrong`, except that this function may
return `false` even if the original value was equal to `expected`. This
may happen if the value could not be updated atomically.

.. note:: `compareExchangeWeak()` is deprecated (and will be repurposed in a
future release), use `compareAndSwap()`.
*/
inline proc compareExchangeWeak(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

/*
Stores `desired` as the new value, if and only if the original value is
equal to `expected`. Returns `true` if `desired` was stored.

.. note:: `compareExchangeStrong()` is deprecated (and will be repurposed in a
future release), use `compareAndSwap()`.
*/
inline proc compareExchangeStrong(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeStrong is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

pragma "no doc"
inline proc const read(order:memory_order): T {
compilerWarning("memory_order is deprecated, use memoryOrder");
Expand Down
50 changes: 32 additions & 18 deletions modules/internal/NetworkAtomics.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,7 @@ module NetworkAtomics {
return ret:bool;
}

inline proc compareExchange(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
return this.compareExchangeStrong(expected, desired, order);
}

inline proc compareExchangeWeak(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
return this.compareExchangeStrong(expected, desired, order);
}

inline proc compareExchangeStrong(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
inline proc compareAndSwap(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
pragma "insert line file info" extern externFunc("cmpxchg", int(64))
proc atomic_cmpxchg(ref expected:int(64), ref desired:int(64), l:int(32), obj:c_void_ptr, ref result:bool(32), order:memory_order): void;

Expand Down Expand Up @@ -117,6 +109,21 @@ module NetworkAtomics {

// Deprecated //

inline proc compareExchange(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchange is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

inline proc compareExchangeWeak(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

inline proc compareExchangeStrong(expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeStrong is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

inline proc const read(order:memory_order): bool {
compilerWarning("memory_order is deprecated, use memoryOrder");
pragma "insert line file info" extern externFunc("read", int(64))
Expand Down Expand Up @@ -249,15 +256,7 @@ module NetworkAtomics {
return ret;
}

inline proc compareExchange(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
return this.compareExchangeStrong(expected, desired, order);
}

inline proc compareExchangeWeak(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
return this.compareExchangeStrong(expected, desired, order);
}

inline proc compareExchangeStrong(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
inline proc compareAndSwap(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
pragma "insert line file info" extern externFunc("cmpxchg", T)
proc atomic_cmpxchg(ref expected:T, ref desired:T, l:int(32), obj:c_void_ptr, ref result:bool(32), order:memory_order): void;

Expand Down Expand Up @@ -379,6 +378,21 @@ module NetworkAtomics {

// Deprecated //

inline proc compareExchange(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchange is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

inline proc compareExchangeWeak(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

inline proc compareExchangeStrong(expected:T, desired:T, param order: memoryOrder = memoryOrder.seqCst): bool {
compilerWarning("compareExchangeStrong is deprecated (and will be repurposed in a future release), use compareAndSwap");
return this.compareAndSwap(expected, desired, order);
}

inline proc const read(order:memory_order): T {
compilerWarning("memory_order is deprecated, use memoryOrder");
pragma "insert line file info" extern externFunc("read", T)
Expand Down
2 changes: 1 addition & 1 deletion modules/packages/AtomicObjects.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ prototype module AtomicObjects {
}

proc compareExchange(expectedObj : objType?, newObj : objType?) : bool {
return atomicVariable.compareExchange(toPointer(expectedObj), toPointer(newObj));
return atomicVariable.compareAndSwap(toPointer(expectedObj), toPointer(newObj));
}

proc compareExchangeABA(expectedObj : ABA(objType?), newObj : objType?) : bool {
Expand Down
2 changes: 1 addition & 1 deletion modules/packages/DistributedBag.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ module DistributedBag {
}

inline proc acquireWithStatus(newStatus) {
return status.compareExchangeStrong(STATUS_UNLOCKED, newStatus);
return status.compareAndSwap(STATUS_UNLOCKED, newStatus);
}

// Set status with a test-and-test-and-set loop...
Expand Down
4 changes: 2 additions & 2 deletions modules/packages/DistributedDeque.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ module DistributedDeque {
on queueSize {
var readSize = queueSize!.read();
// Attempt to fix, but yield to reduce potential contention and CPU hogging.
while readSize < 0 && !queueSize!.compareExchangeWeak(readSize, 0) {
while readSize < 0 && !queueSize!.compareAndSwap(readSize, 0) {
chpl_task_yield();
readSize = queueSize!.read();
}
Expand Down Expand Up @@ -426,7 +426,7 @@ module DistributedDeque {
on queueSize {
var readSize = queueSize!.read();
// Attempt to fix, but yield to reduce potential contention and CPU hogging.
while readSize > this.cap && !queueSize!.compareExchangeWeak(readSize, this.cap) {
while readSize > this.cap && !queueSize!.compareAndSwap(readSize, this.cap) {
chpl_task_yield();
readSize = queueSize!.read();
}
Expand Down
9 changes: 9 additions & 0 deletions test/deprecated/compareExchange.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
var b: atomic bool; writeln(b);
b.compareExchange(false, true); writeln(b);
b.compareExchangeStrong(true, false); writeln(b);
while !b.compareExchangeWeak(false, true) { } writeln(b);

var i: atomic int; writeln(i);
i.compareExchange(0, 1); writeln(i);
i.compareExchangeStrong(1, 2); writeln(i);
while !i.compareExchangeWeak(2, 3) { } writeln(i);
16 changes: 16 additions & 0 deletions test/deprecated/compareExchange.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
compareExchange.chpl:2: warning: compareExchange is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:3: warning: compareExchangeStrong is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:4: warning: compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:4: warning: compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:7: warning: compareExchange is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:8: warning: compareExchangeStrong is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:9: warning: compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap
compareExchange.chpl:9: warning: compareExchangeWeak is deprecated (and will be repurposed in a future release), use compareAndSwap
false
true
false
true
0
1
2
3
2 changes: 1 addition & 1 deletion test/exercises/boundedBuffer/boundedBuffer.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ proc consumer(b: BoundedBuffer, cid: int) {
// ... put your code to initialize the atomics here ...
// }
//
// (2) read(), write() and compareExchange() are going to be the most
// (2) read(), write() and compareAndSwap() are going to be the most
// useful methods on atomics for this exercise. If you haven't worked
// with atomics before, refer to the online documentation or ask one
// of the helpers for a hint:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class BoundedBuffer {
do {
prevPos = pos.read();
const nextPos = (prevPos + 1) % capacity;
} while (!pos.compareExchange(prevPos, nextPos));
} while (!pos.compareAndSwap(prevPos, nextPos));

return prevPos;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,10 @@ proc blocking_loop_int_read() {
}
blocking_loop_int_read();

proc blocking_loop_int_compare_exchange() {
while globalAtomicInt.compareExchange(0, 1) == false { }
proc blocking_loop_int_compare_and_swap() {
while globalAtomicInt.compareAndSwap(0, 1) == false { }
}
blocking_loop_int_compare_exchange();

proc blocking_loop_int_compare_exchange_weak() {
while globalAtomicInt.compareExchangeWeak(0, 1) == false { }
}
blocking_loop_int_compare_exchange_weak();
blocking_loop_int_compare_and_swap();

proc blocking_loop_int_waitfor() {
globalAtomicInt.waitFor(false);
Expand All @@ -34,15 +29,10 @@ proc blocking_loop_bool_read() {
}
blocking_loop_bool_read();

proc blocking_loop_bool_compare_exchange() {
while globalAtomicBool.compareExchange(false, true) == false { }
}
blocking_loop_bool_compare_exchange();

proc blocking_loop_bool_compare_exchange_weak() {
while globalAtomicBool.compareExchangeWeak(false, true) == false { }
proc blocking_loop_bool_compare_and_swap() {
while globalAtomicBool.compareAndSwap(false, true) == false { }
}
blocking_loop_bool_compare_exchange_weak();
blocking_loop_bool_compare_and_swap();

proc blocking_loop_bool_test_and_set() {
while globalAtomicBool.testAndSet() == true { }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
blocking-analysis1.chpl:12: note: atomics,blocking blocking_loop_int_read
blocking-analysis1.chpl:17: note: atomics,blocking blocking_loop_int_compare_exchange
blocking-analysis1.chpl:22: note: atomics,blocking blocking_loop_int_compare_exchange_weak
blocking-analysis1.chpl:27: note: yields-tasks,atomics,blocking blocking_loop_int_waitfor
blocking-analysis1.chpl:32: note: atomics,blocking blocking_loop_bool_read
blocking-analysis1.chpl:37: note: atomics,blocking blocking_loop_bool_compare_exchange
blocking-analysis1.chpl:42: note: atomics,blocking blocking_loop_bool_compare_exchange_weak
blocking-analysis1.chpl:47: note: atomics,blocking blocking_loop_bool_test_and_set
blocking-analysis1.chpl:52: note: yields-tasks,atomics,blocking blocking_loop_bool_waitfor
blocking-analysis1.chpl:58: note: anything blocking_sync_write
blocking-analysis1.chpl:63: note: anything blocking_sync_read
blocking-analysis1.chpl:69: note: anything blocking_single_write
blocking-analysis1.chpl:74: note: anything blocking_single_read
blocking-analysis1.chpl:81: note: atomics,blocking blocking_in_if
blocking-analysis1.chpl:88: note: atomics,blocking blocking_in_else
blocking-analysis1.chpl:97: note: atomics,blocking blocking_nested_loop
blocking-analysis1.chpl:17: note: atomics,blocking blocking_loop_int_compare_and_swap
blocking-analysis1.chpl:22: note: yields-tasks,atomics,blocking blocking_loop_int_waitfor
blocking-analysis1.chpl:27: note: atomics,blocking blocking_loop_bool_read
blocking-analysis1.chpl:32: note: atomics,blocking blocking_loop_bool_compare_and_swap
blocking-analysis1.chpl:37: note: atomics,blocking blocking_loop_bool_test_and_set
blocking-analysis1.chpl:42: note: yields-tasks,atomics,blocking blocking_loop_bool_waitfor
blocking-analysis1.chpl:48: note: anything blocking_sync_write
blocking-analysis1.chpl:53: note: anything blocking_sync_read
blocking-analysis1.chpl:59: note: anything blocking_single_write
blocking-analysis1.chpl:64: note: anything blocking_single_read
blocking-analysis1.chpl:71: note: atomics,blocking blocking_in_if
blocking-analysis1.chpl:78: note: atomics,blocking blocking_in_else
blocking-analysis1.chpl:87: note: atomics,blocking blocking_nested_loop
blocking-analysis1.chpl:9: note: computed main
Loading