And just to add to the delay, I've been off sick...
First of all - thanks for the patch!
The patch adds support for currently unsupported platforms, without actually
changing any code paths for currently supported platforms. So from that
perspective, I would not object strongly to it being applied as is.
However, I do have a few minor comments on the code:
- The v5 code doesn't actually make use of the kuser helper barriers
in its versions of opal_atomic_cmpset_acq/rel.
- The line
+#if (OMPI_GCC_INLINE_ASSEMBLY || (OPAL_ASM_ARM_VERSION < 6))
is correct, but does my head in. Could another define like
OPAL_ARM_KUSER_BARRIERS or similar be added by the barrier definition
and used here instead, to improve readability?
- Could you change the line
+#if (OPAL_ASM_ARM_VERSION >= 6 && OMPI_GCC_INLINE_ASSEMBLY)
+#if (OMPI_GCC_INLINE_ASSEMBLY && (OPAL_ASM_ARM_VERSION >= 6))
again for readability?
And a few higher-level comments/suggestions:
- The patch does not do a runtime test for kernel helper version. While
normally not a problem, this could cause tricky to debug issues if
running on top of old kernels (as in "executing uninitialized
Not sure what the best way to hook such a test in would be though.
- This patch does not include out-of-line assembly versions
(in opal/asm/base) for the atomic operations. However I am not sure
if this causes any practical problems.
- If 64-bit atomics are desirable, these are actually possible on most
ARMv6 platforms (including the Raspberry PI), so a configure test on
whether LDREXD assembles without errors could be used to detect this.
Longer-term, I'd like to migrate to using the new GCC __atomic* intrinsics
(in 4.7 - http://gcc.gnu.org/wiki/Atomic/GCCMM). However, the old __sync*
intrinsic were a bit heavyweight so until 4.7 is ubiquitous I prefer to keep
the inline asm, and now the kuser calls.
> -----Original Message-----
> From: Jeffrey Squyres [mailto:jsquyres_at_[hidden]]
> Sent: 19 April 2012 16:21
> To: Open MPI Developers; Leif Lindholm
> Subject: Re: [OMPI devel] [PATCH] Open MPI on ARMv5
> Thanks Evan!
> (sorry for the delay in replying -- I was on vacation all last week and
> I'm *still* catching up...)
> Lief -- does this look good to you?
> On Apr 13, 2012, at 11:13 PM, Evan Clinton wrote:
> > At present Open MPI only supports ARMv7 processors. Attached is a
> > patch against current trunk (r26270) that extends the atomic
> > operations and memory barriers code to work with ARMv5 and ARMv6
> > too.
> > For v6, the only changes were to use "mcr p15, 0, r0, c7, c10, 5"
> > instead of the unavailable DMB instruction, and to disable the 64 bit
> > compare-exchange function (which I understand is not vital for Open
> > MPI on 32 bit platforms?). For v5, it was a bit trickier; the
> > processor lacks nice memory barrier instructions or proper atomic
> > operations. Fortunately, the Linux kernel offers several helper
> > functions on ARM, and I've used those here.
> > The changes build and pass all of the assembly-related tests in the
> > test folder and the hello world examples run on my "armv5tel" box
> > running Debian with Linux 2.6.32-5. It should also run fine on ARMv6
> > boxes, and presumably v4, but I don't have either to test on.
> > Documentation for the Linux kernel helper functions:
> > I've sent in a contributor agreement so there should be no IP
> > Hopefully this is useful,
> > Evan Clinton
> > <ompi_armv5.diff>_______________________________________________
> > devel mailing list
> > devel_at_[hidden]
> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Jeff Squyres
> For corporate legal information go to: