Different ways of implementing atomic *=

Question

Studying through a book, it explains how to implement more complex operations like operator* for std::atomic<T>. Implementation uses compare_exchange_weak and I think I understood how this works. Now, I implemented things myself, take a look.

#include <type_traits>
#include <atomic>
#include <iostream>

/*template <typename T, typename = std::enable_if_t<std::is_integral<T>::value>>
std::atomic<T>& operator*=(std::atomic<T>& t1, T t2) {
    T expected = t1.load();
    while(!t1.compare_exchange_weak(expected, expected * t2))
    {}
    return t1;
}*/

template <typename T, typename = std::enable_if_t<std::is_integral<T>::value>>
std::atomic<T>& operator*=(std::atomic<T>& t1, T t2) {
    T expected = t1.load();
    t1.compare_exchange_weak(expected, expected * t2);
    return t1;
}

int main() {
    std::atomic<int> t1 = 5;
    std::atomic<int> t2;
    t2 = (t1 *= 5).load();

    std::cout << "Atomic t1: " << t1 << "\n";
    std::cout << "Atomic t2: " << t2 << "\n";
} 

I have two versions of the code, book's version is commented out. I don't get why I should wait on a busy-loop to perform atomic compare_exchange. In my version, I've just called it on its own line and looking at the generated assembly in Godbolt, both uses

lock cmpxchg dword ptr [rsp + 8], ecx

and looks pretty similar to me. So, why should I need a wait-loop like the one in the book to make this thing atomic? Isn't my version also fine and do work atomically?

Answer 1

Imagine between your call to load and compare_exchange_weak the value gets changed by another thread. expected has no longer the current value.

compare_exchange_weak works as follows:

Atomically compares the (object representation (until C++20)/ value representation (since C++20)) of *this with that of expected, and if those are bitwise-equal, replaces the former with desired (performs read-modify-write operation). Otherwise, loads the actual value stored in *this into expected (performs load operation). cppreference

Based on the description above t1 would not be altered and your multiplication would not be stored. By looping you ensure to either update t1 and store the result of the multiplication or to update expected and to try again in the next iteration of the loop (the loop does only stop once the first case occurred).

---

EDIT: You can "try" it by simulating the concurrent access. Before exchanging the result another thread comes in and changes the value of the atomic. In the following the compare_exchange_weak only affects expected.

+----------- Thread 1 -----------+---------- Thread 2 ----------+
| ex = t1.load()                 |                              |
|                                | t1.store(42)                 |
| t1.cmp_xchg_w(ex, ex * t2)     |                              |

This code simulates the concurrent access and letting individual threads sleep.

#include <type_traits>
#include <atomic>
#include <iostream>
#include <chrono>
#include <thread>

template <typename T, typename = std::enable_if_t<std::is_integral<T>::value>>
std::atomic<T>& operator*=(std::atomic<T>& t1, T t2) {
    using namespace std::chrono_literals;
    T expected = t1.load();
    std::this_thread::sleep_for(400ms);
    t1.compare_exchange_weak(expected, expected * t2);
    return t1;
}

int main() {
    std::atomic<int> t1 = 5;
    std::atomic<int> t2;
    std::thread th1([&](){
        t2 = (t1 *= 5).load();
    });
    std::thread th2([&](){
        using namespace std::chrono_literals;
        std::this_thread::sleep_for(100ms);
        t1.store(8);
    });

    th1.join();
    th2.join();

    std::cout << "Atomic t1: " << t1 << "\n";
    std::cout << "Atomic t2: " << t2 << "\n";
}