Comment 5 for bug 490371

Revision history for this message
Dave Martin (dave-martin-arm) wrote :

This patch adds memory barriers to the existing armv6-based atomics implementation.

This is a quicker interim fix than porting the code to use the GCC atomics, but that could still be addressed later.

The change is conservative: a barrier is added both before and after the body of each atomic. This is suboptimal: Qt's generic atomics API (see src/corelib/thread/qatomic.cpp) defines 4 flavours of each atomic with differing barrier requirements, but they are not all implemented for ARM. The code is in need of a substantional factoring before this is attempted: it would raise the number of duplicates of each code snippet in the source from ~10 to ~40 rendering reliable maintenance all but impossible. I'm not sure whether the preprocessor is up to the job, but the amount of hard coding could be massively reduced by generating the affected files using a script.

The existing code implements the "Relaxed" (no barrier needed), "Acquire" (requires barrier after) and "Release" (requires barrier before) flavours in terms of the "Ordered" implementation (barriers required before and after).

The attached patch is required to fix the "Ordered" implementation so that it really does implement the Ordered semantics: without the barriers, this obtained behaviour is actually the "Relaxed" behaviour with respect to the memory system, which may lead to problems in SMP environments.

The proposed patch applies and builds, but I haven't been able to test it yet (huge lead time on building the qt4 packages...) I will post a follow-up when I have a full build of the affected packages.