Is this C++ AtomicInt implementation correct?

Question

Premise: I'm working with an ARM embedded (almost bare-metal) environment where I don't even have C++11 (with std::atomic<int>) available, so please avoid answers like "just use standard C++ std::atomic<int>": I can't.

Is this ARM implementation of AtomicInt correct? (assume ARM architecture is ARMv7-A)

Do you see some synchronization issue? Is it volatile required/useful?

// File: atomic_int.h

#ifndef ATOMIC_INT_H_
#define ATOMIC_INT_H_

#include <stdint.h>

class AtomicInt
{
public:
    AtomicInt(int32_t init = 0) : atom(init) { }
    ~AtomicInt() {}

    int32_t add(int32_t value); // Implement 'add' method in platform-specific file

    int32_t sub(int32_t value) { return add(-value); }
    int32_t inc(void)          { return add(1);      }
    int32_t dec(void)          { return add(-1);     }

private:
    volatile int32_t atom;
};

#endif

// File: arm/atomic_int.cpp

#include "atomic_int.h"

int32_t AtomicInt::add(int32_t value)
{
    int32_t res, prev, tmp;

    asm volatile(

    "try:    ldrex   %1, [%3]\n"     // prev = atom;
    "        add     %0, %1, %4\n"   // res = prev + value;
    "        strex   %2, %0, [%3]\n" // tmp = outcome(atom = res); // may fail
    "        teq     %2, #0\n"       // if (tmp)
    "        bne     try"            //     goto try; /* add failed: someone else modified atom -> retry */

    : "=&r" (res), "=&r" (prev), "=&r" (tmp), "+mo" (atom)  // output (atom is both in-out)
    : "r" (value)                                           // input
    : "cc");                                                // clobbers (condition code register [CPSR] changed)

    return prev; // safe return (local variable cannot be changed by other execution contexts)
}

Also, I'm trying to achieve some code reuse, that is why I isolated just one basic function to implement in platform-specific code (add() method inside arm/atomic_int.cpp).

Is atomic_int.h really portable as it is across different platforms/architectures/compilers? Is this approach feasible? (With feasible I mean feasible for every platform to guarantee atomicity by implementing just the add() method).

here is the corresponding ARM GCC 8.3.1 implementation of the same function. Apparently, the only real difference is the presence of dmb before and after. Are they really needed in my case? Why? Do you have an example where my AtomicInt (without dmb) fails?

UPDATE: fixed implementation, removed get() method to solve atomicity and alignment issues. Now the add() behaves like a standard fetchAndAdd().

Answer 1

If you use gcc may be you can use Legacy __sync Built-in Functions for Atomic Memory Access:

void add(int volatile& a, int value) {
    __sync_fetch_and_add(&a, value);
}

Generates:

add(int volatile&, int):
.L2:
        ldxr    w2, [x0]
        add     w2, w2, w1
        stlxr   w3, w2, [x0]
        cbnz    w3, .L2
        dmb     ish
        ret