Lock-free SPMC queue

Here is my lock-free queue implementation for single producer with some preallocated memory. T is a simple type with no need for move operations. I don't use bool pop(T& result) in order to avoid double checking. I would love to see your feedback!

template 
class LockFreeQueue {
public:
    void setup(U8* buffer, U32 size) {
        m_buffer = (T*)buffer;
        m_size = size;
        clear();
    }

    void clear() {
        m_first.store(0, std::memory_order_relaxed);
        m_last.store(0, std::memory_order_relaxed);
    }

    void push(const T& value) {   // only for single producer
        U32 last = m_last.load(std::memory_order_relaxed);
        m_buffer[last] = value;
        U32 next = inc(last);
        ASSERT(next != m_first.load(std::memory_order_relaxed), "Queue overflow");
        m_last.store(next, std::memory_order_release);
    }

    bool isEmpty() {
        return m_first.load(std::memory_order_relaxed) == m_last.load(std::memory_order_relaxed);
    }

    void pop(T& result) {    // for single consumer. check isEmpty() before
        ASSERT(!isEmpty(), "Queue is empty");
        U32 first = m_first.load(std::memory_order_relaxed);
        result = m_buffer[first];
        m_first.store(inc(first), std::memory_order_release);
    }

    bool tryPop(T& result) {    // for multiple consumers. don't use isEmpty()
        while (true) {
            U32 first = m_first.load(std::memory_order_relaxed);
            if (first == m_last.load(std::memory_order_relaxed))
                return false;
            result = m_buffer[first];
            if (m_first.compare_exchange_weak(first, inc(first), std::memory_order_release))
                return true;
        }
    }

private:
    U32 inc(U32 n) { return (n + 1) % m_size; }

    T* m_buffer;
    U32 m_size;
    std::atomic m_first;
    std::atomic m_last;
};

Memory ordering problem

This line:

if (first == m_last.load(std::memory_order_relaxed))
            return false;

should be:

if (first == m_last.load(std::memory_order_acquire))
            return false;

Otherwise, on the next line you may load the wrong value out of m_buffer due to the load being reordered before the load to m_last. I spotted this because I noticed a memory_order_release without a corresponding memory_order_acquire. The pop() function also needs a load of m_last with memory_order_acquire.

You have the same problem in push(). If you don't load m_first with memory_order_acquire, you run the risk of writing to the buffer when it is full, thus overwriting some element. Even the assertion won't help because you could have overwritten the buffer and then loaded a modified value for m_first.
Example of failure

Since your comment indicated that I didn't explain thoroughly enough, I will give an example of how the pop could go wrong.

• The queue is empty, with m_first = m_last = 0.

• The consumer calls try_pop().

• In try_pop(), m_first is loaded as 0.

• Now, the cpu speculatively loads m_buffer[0], even though you haven't reached that line yet.

• The producer calls push().

• The producer writes m_buffer[0], and then increments m_last to 1, in that order.

• The consumer reads m_last as 1.

• The consumer uses the value speculatively loaded in step 4 as the value for m_buffer[0], but this value is wrong.

By adding the memory_order_acquire, you prevent the load of m_buffer[0] from happening until after step 7.

StackExchange Code Review Q#119367, answer score: 6