updating maximum value atomically

Question

Is the following code a correct implementation to update the maximum value for an atomic variable? Is the usage of std::memory_order_relaxed for both success and failure correct and optimal?

template<typename T>
inline void update_max(std::atomic<T> & atom, const T val)
{
  for(T atom_val=atom;
      atom_val < val &&
      !atom.compare_exchange_weak(atom_val, val, std::memory_order_relaxed);
     );
}

Note that this question addressed essentially the same problem (though in a particular context), but the (accepted) answer is not conclusive, in particular regarding the memory order, (and potentially out of date).

Answer 1

The strategy used to atomically update max value in a thread safe manner is correct.

Whether or not memory ordering is correct is impossible to tell because of code you are not showing. If the atomic max value isn't used in any context other than reporting a value (i.e. no dependencies on other memory operations), you'll probably get away with std::memory_order_relaxed.

As I mentioned in my comment, on X86 the compiler is likely to produce the same assembly instructions regardless the use of memory ordering parameters. X86 is a strongly ordered CPU which means that (by default) #LoadLoad and #LoadStore reordering is not allowed. therefore you won't find a (sane) compiler that will issue a memory fence around a seq_cst load. (#StoreLoad reordering is still allowed by default, but to prevent that for seq_cst ordering is typically handled at the store side).

As for compare_exchange_weak (a read-modify-write operation), this requires the cache line to be locked in order to be atomic; you will see these assembly instructions on X86: lock cmpxchg
Since this also serves as a full memory barrier, it eliminates the need for additional fences.

Note that if you use std::memory_order_relaxed on any atomic operation, the compiler still has the freedom to apply compile time reordering