Chapel: Finalize compareExchange memory orders

[ronawho]

With https://github.com/chapel-lang/chapel/pull/14883 we have implemented a compareExchange that is closer to C/C++ in that expected is passed by ref and updated on failure. However, we're missing the version that takes both a success and failure memory order:

proc compareExchangeStrong(ref expected:bool, desired:bool, param order: memoryOrder = memoryOrder.seqCst): bool

bool compare_exchange_strong(T& expected, T desired, std::memory_order success, std::memory_order failure);
bool compare_exchange_strong(T& expected, T desired, std::memory_order order = std::memory_order_seq_cst);

We currently only have the single order case (where the failure order is automatically determined.) We should support the case where a success and failure order are taken too, since this can offer better performance. I think we have 2 options -- we can either provide an overload that takes both arguments like C++, or we could provide a single version whose failure argument is based on the success order. So either:

proc compareExchange(..., param order: memoryOrder = memoryOrder.seqCst): bool
proc compareExchange(..., param success: memoryOrder, param failure: memoryOrder): bool

or

proc compareExchange(..., param success: memoryOrder = memoryOrder.seqCst, param failure: memoryOrder = readableOrder(success)): bool

Unlike C++, Chapel supports default arguments that are based on earlier arguments so it's possible for us to just have 1 version like this. If we go this route we probably need a better name than readableOrder.

I have a slight preference for the 2 overloads.

[ronawho]

Vote for 2 overloads:

proc compareExchange(..., param order: memoryOrder = memoryOrder.seqCst): bool
proc compareExchange(..., param success: memoryOrder, param failure: memoryOrder): bool

[ronawho]

Vote for 1 version (suggestions for readableOrder name are welcome):

proc compareExchange(..., param success: memoryOrder = memoryOrder.seqCst, param failure: memoryOrder = readableOrder(success)): bool

[e-kayrakli]

I really don't like "1 version" option. If I don't care too much about optimizing different cases w.r.t order, yet like to use named arguments (I really like them personally and use very liberally) I'd have to write compareExchange(..., success=memoryOrder.seqCst). The argument name "success" doesn't look good to me here. Being able to say "order" would be much more expressive and much less confusing. Even more so if I do this a lot and have use memoryOrder somewhere in my code: success=seqCst wouldn't mean much for someone who's reading this code.

[ronawho]

No matter what version we go with a user will still be able to write:

a.compareExchange(expected, desired); 
a.compareExchange(expected, desired, memoryOrder.seqCst);
a.compareExchange(expected, desired, memoryOrder.seqCst, memoryOrder.seqCst);

So I think this is more of a documentation (and argument name) question. Most users will likely not specify a memory order (a.compareExchange(expected, desired)), and get seqCst ordering. For users who know enough about memory orders to feel comfortable using them, I would guess they will end up using the 2 argument version.

So I think the main benefit of the 2-overload version is that if you only want to specify just 1 order, and you like named arguments you can do:

a.compareExchange(expected, desired, order=memoryOrder.seqCst);

Instead of:

a.compareExchange(expected, desired, success=memoryOrder.seqCst);

Which I agree is clearer.

For either approach if you're using the 2 order version and like named arguments it would still look like:

a.compareExchange(expected, desired, success=memoryOrder.seqCst, failure=memoryOrder.seqCst);

Both C++ and Rust use success/failure, which I agree aren't very descriptive. We could do something like successOrder, but that starts to get a little long.

[e-kayrakli]

I agree that this only matters if you use explicit argument names. But I do that a lot as a stylistic matter :)

I am fine with success/failure. I see them as advanced-user-oriented arguments (even order is, to some extent), so maybe conciseness is better than expressiveness. Similarity with other languages is also good.

[bradcray]

No real preference here.

[gbtitus]

I agree successOrder seems long, but on the other hand just plain success seems like it doesn't give enough information about what it is. Maybe succOrder and failOrder? It's worth spending a little effort to get these right, because I suspect the named-argument usage will be the common one, for clarity. That is, people won't just write a.compareExchange(expected, desired, memoryOrder.someOrder, memoryOrder.someOtherOrder); they'll use the formalParam= form.

[ronawho]

Rust and C/C++ use success/failure so the names don't bother me.

For something like:

a.compareExchange(expected, desired, succes=memoryOrder.seqCst, failure=memoryOrder.relaxed);

the enum name makes it clear that it's a memory order. A user could also write:

a.compareExchange(expected, desired, succes=seqCst, failure=relaxed);

but that still seems pretty clear to me.

[gbtitus]

The fact that in named-argument usage the actual would be memoryOrder.whatever value does help, and I haven't been able to come up with alternative formal parameter names that I _really_ like, so it's okay with me if we go with plain old success and failure for now. If something better occurs to me I'll raise my hand. :-)