Reputation: 65
I have some code that copies masked bits into an integer by first clearing them in the target int then ORing them into the int.
Like this:
bitsToSet = 6
targetInt &= ~(1 << bitsToSet)
targetInt |= desiredBitValue << bitsToSet
The problem is that it now needs to be thread safe, and I need to make the operation atomic. I think that using std:atomic<int> would only make each sub-operation atomic but not the operation as a whole.
How can I make the whole operation (including both the &= and the |= operations) atomic?
For example, it would solve the problem for me if I had a function (or better, a macro) like SetBits(TARGET, MASK, VALUE)
that would atomically set the MASKed bits in TARGET to VALUE. The MASK and VALUE can already be left-shifted.
My current, non-atomic code is
#define SetBits(TARGET, MASK, VALUE) {(TARGET) &= ~((uint64_t)MASK); (TARGET)|=((uint64_t)VALUE);}
Upvotes: 4
Views: 488
Reputation: 50806
Copying a bit can be done atomically without a CAS, but at the expense of a conditional code. Indeed, setting a bit can be done with an atomic or and clearing a bit can be done with an atomic and. The trick is to combine them together:
bitToSet = 7;
// Assume targetInt is of type std::atomic<int>
if(desiredBitValue)
targetInt.fetch_or(1 << bitToSet);
else
targetInt.fetch_and(~(1 << bitToSet));
For multiple bits, the only portable way to do that in C++ so far is to use a compare_exchange_weak
operation on targetInt
in a loop.
Upvotes: 4
Reputation: 1
Use std::mutex
. Your code should look like this:
mutex m;
void SetBitsAtomic()
{
m.lock();
bitToSet = 7;
targetInt &= ~(1 << bitToSet);
targetInt |= desiredBitValue << bitToSet;
m.unlock();
}
Upvotes: 1
Reputation: 30579
You could use a compare-exchange loop:
void SetBitsAtomic(std::atomic<int>& target, int mask, int value) {
int original_value = target.load();
int new_value = original_value;
SetBits(new_value, mask, value);
while (!target.compare_exchange_weak(original_value, new_value)) {
// Another thread may have changed target between when we read
// it and when we tried to write to it, so try again with the
// updated value
new_value = original_value;
SetBits(new_value, mask, value);
}
}
This reads the original value of target
, does the masking operations, and then writes the modified value back to target
only if no other thread has modified it since it was read. If another thread has modified target
then its updated value gets written into original_value
and it keeps trying until it manages to update target
before anyone else.
Note that I've used (default) full sequential consistency here for the load
and compare_exchange_weak
operations. You may not need full sequential consistency, but I have no way of knowing exactly what you need without more info on what you're using this for.
Alternatively, you could just use a mutex:
std::mutex mtx;
void SetBitsAtomic(int& target, int mask, int value) {
std::lock_guard lock{mtx};
SetBits(target, mask, value);
}
This may be less performant than the lock-free compare_exchange_weak
version, but again it really depends what it's being used for. It's certainly simpler and easier to reason about, which may be more important than raw performance in your situation.
Upvotes: 5