includes/stg/SMP.h: implement simple load_/store_load_barrier on armv6 and older
ClosedPublic

Authored by trofi on May 17 2015, 7:43 AM.

Details

Summary

Assuming there is no real SMP systems on these CPUs
I've added only compiler barrier (otherwise write_barrier
and friends need to be fixed as well).

Patch also fixes build breakage reported in Trac #10244.

Signed-off-by: Sergei Trofimovich <siarheit@google.com>

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
T10244-armv6-threaded
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4050
Build 4081: GHC Patch Validation (amd64/Linux)
trofi updated this revision to Diff 2954.May 17 2015, 7:43 AM
trofi retitled this revision from to includes/stg/SMP.h: implement simple load_/store_load_barrier on armv6 and older.
trofi updated this object.
trofi edited the test plan for this revision. (Show Details)
trofi added reviewers: rwbarton, nomeata.
trofi updated the Trac tickets for this revision.
nomeata accepted this revision.May 17 2015, 7:55 AM
nomeata edited edge metadata.

This patch has already been applied to the Debian package, looks good so far.

This revision is now accepted and ready to land.May 17 2015, 7:55 AM
austin accepted this revision.May 18 2015, 3:25 PM
austin edited edge metadata.

LGTM.

This revision was automatically updated to reflect the committed changes.
rwbarton edited edge metadata.May 18 2015, 4:48 PM

I don't think it is so simple. I don't know whether there are actually SMP pre-ARMv7 systems, but arm_HOST_ARCH_PRE_ARMv7 only means that we are targeting a pre-ARMv7 instruction set, but the resulting executables can also run on later versions of the ARM architecture.

For example, the ghc-android (https://github.com/neurocyte/ghc-android) build sets arm_HOST_ARCH_PRE_ARMv7 (and even arm_HOST_ARCH_PRE_ARMv6), which I assume means that executables it produces can even run on ARMv5, but they also run on my tablet, which is an SMP ARMv7 system.

I'm speculating here, but I guess that programs that implement their own concurrency primitives for SMP are supposed to detect at runtime whether they need to use the memory barriers that are only available on ARMv7 (if that is actually the case). If we don't want to go that far, then possibly it would make sense to only support WITH_SMP when !defined(arm_HOST_ARCH_PRE_ARMv7).

I'm fine with this patch since it is better than failing to build completely, but we should at least have some kind of FIXME comment since the code seems likely to be wrong.

trofi added a comment.May 19 2015, 4:32 AM

I completely agree. I suggest rewording Ticket Trac #10244 (or create a new one?) to a form that
it's explicitly not only about build failure but about general issue of sprinkling 'dmb' instruction at
compile time.