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

cmpxchg function is missing the loaded value #461

Closed
andrewrk opened this issue Sep 10, 2017 · 4 comments
Closed

cmpxchg function is missing the loaded value #461

andrewrk opened this issue Sep 10, 2017 · 4 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Milestone

Comments

@andrewrk
Copy link
Member

@cmpxchg(ptr: &T, cmp: T, new: T, success_order: AtomicOrder, fail_order: AtomicOrder) -> bool

cmpxchg is supposed to be a read-modify-write atomic operation. It should provide a way to return the read value.

It should return ?T instead:

@cmpxchg(ptr: &T, cmp: T, new: T, success_order: AtomicOrder, fail_order: AtomicOrder) -> ?T
@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. enhancement Solving this issue will likely involve adding new logic or components to the codebase. labels Sep 10, 2017
@andrewrk andrewrk added this to the 0.1.0 milestone Sep 10, 2017
@andrewrk
Copy link
Member Author

I need to remind myself of the use of cmpxchg, but it looks like the case where you actually need to use T is the failure case. So maybe just return an anonymous struct:

@cmpxchg(ptr: &T, cmp: T, new: T, success_order: AtomicOrder, fail_order: AtomicOrder) -> struct { success: bool, old_value: T}

@andrewrk
Copy link
Member Author

Let's make sure these compile errors are in place too:

The type of ‘cmp’ must be an integer or pointer type whose bit width is a power of two greater than or equal to eight and less than or equal to a target-specific size limit.

@kyle-github
Copy link

kyle-github commented Sep 12, 2017

Alternatively, you could return the value that was there (this is what similar calls do in Windows and Linux).

@cmpxchg(ptr: &T, cmp: T, new: T, success_order: AtomicOrder, fail_order: AtomicOrder) -> %T

I suggest modifying this to have the return type be %T. It is either an error (for instance bad alignment) or you get a valid T value. If the T value is the old value (cmp), then the swap succeeded. If the T value is something else, then the swap failed.

const old = %return @cmpxchg(myTPtr, oldVal, newVal, ?, ?);

if(old == oldVal) {
   ... successful swap ...
else {
   ... failure ...
}

(Have I mentioned, lately, how much I like the error idea in Zig?)

@kyle-github
Copy link

Oops, forgot to comment about the size. All processors that support some form of CAS ("compare and swap" is the generic way this is referenced in the literature) usually only support a very small number of sizes so this is inherently a platform specific operation. Virtually all CPUs that support it at all support at least a single machine word. Some support DCAS (double compare and swap) and a few support QCAS (quad compare and swap). I am not aware of any that support some form of CAS that is not single machine word at least.

Where things get a little frustrating is that "machine word" is not a very solid thing. Is that the size of an address? Of an integer register?

I do not have any good answers here, but I usually assume that you can swap an address IFF the size of an address is something relatively sane. I.e. Intel's weird addressing modes with explicit segments do not work.

@andrewrk andrewrk modified the milestones: 0.1.0, 0.2.0 Sep 18, 2017
@andrewrk andrewrk modified the milestones: 0.2.0, 0.3.0 Jan 3, 2018
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Feb 28, 2018
@andrewrk andrewrk mentioned this issue Apr 16, 2018
8 tasks
@andrewrk andrewrk modified the milestones: 0.4.0, 0.3.0 Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Projects
None yet
Development

No branches or pull requests

2 participants