Differential D61052
[compiler-rt][builtins] Implement some fetch-and-x operations for Cortex-M Authored by aykevl on Apr 23 2019, 6:57 PM.
Details
This patch adds support for the following builtins on the Cortex-M0 and up: __sync_fetch_and_add_4 __sync_fetch_and_add_8 __sync_fetch_and_and_4 __sync_fetch_and_and_8 __sync_fetch_and_nand_4 __sync_fetch_and_nand_8 __sync_fetch_and_or_4 __sync_fetch_and_or_8 __sync_fetch_and_sub_4 __sync_fetch_and_sub_8 __sync_fetch_and_xor_4 __sync_fetch_and_xor_8 The assembly for Cortex-M3 and up could be slightly more efficient, but I've chosen to keep it simple so that all ARM processors can deal with it.
Diff Detail
Event TimelineComment Actions If someone could suggest a good way to test this, that would be great. At the moment it has been tested by compiling for armv6m, armv7m, and armv7em and confirming that the add operations appear to work. Comment Actions Is there any reason to avoid 32-bit ldrex/strex on M-class processors other than Cortex-M0? I haven't really done any systems programming on ARM, so I'm not sure what the consequences of the sequence you use for enabling/disabling interrupts might be. Is it reasonable for a library to turn off interrupts for a few cycles? Is it possible that the value of primask might change after the mrs, but before the cpsid instruction? Comment Actions No there isn't any reason to avoid ldrex/strex on M-class. It is the recommended way of implementing these functions on all architectures that support them.
It depends on your application. If the application requires hard real time low latency guarantees for interrupt response time but also uses concurrency elsewhere for non-real time parts then disabling interrupts for these functions could be a serious problem. You typically wouldn't use a cortex-m0 for such an application but a cortex-m3/m4 is certainly possible. Personally I think that we should be using ldrex/strex on all architectures that support them.
In theory
I'm struggling to think of a use case of an asynchronous interrupt that would disable further interrupts and rely on some other (non interrupt) part of the code to re-enable them. One other point, are these files compiled for cortex-m0? My reading of builtins/CMakeLists.txt with: set(thumb1_SOURCES
arm/divsi3.S
arm/udivsi3.S
arm/comparesf2.S
arm/addsf3.S
${GENERIC_SOURCES}
)
...
set(thumb1_SOURCES
${thumb1_SOURCES}
${arm_EABI_SOURCES}
)
...
set(armv6m_SOURCES ${thumb1_SOURCES})The sync_fetch_and_add_4.S etc. are only defined for arm_Sources which are v7 and above. If I'm right you'd need to alter the build-system for v6m for these files to be included in the library? Am I missing something? Comment Actions
Ok, I'll update the code to use ldrex/strex on armv7m and up for 32-bit operations. I don't see a way of using ldrex/strex for 64-bit operations on any of the Cortex-M devices, though (unless the more capable ones have 64-bit ldrex/strex instructions).
I have been thinking about this as well but if the only possible implementation is by disabling interrupts, I think we should do that. If anyone knows a better way on Cortex-M0 please tell.
That was my thinking as well. Although it is possible, I think it would be bad practice. Many RTOSes have macros for disabling/enabling interrupts using the same pattern.
Good point, will take a look. I haven't used the CMake-based built system for my application so I didn't need it but will make sure it works with it. Comment Actions
Comment Actions Thanks for the update. While it will be difficult to test for threading problems, it would be possible to test these functions in a single threaded context by adding test cases in compiler-rt/test/builtins/Unit. We'd need to be careful that these only ran when the functions are supported though. Getting these tests to run for native builds would be simple, cortex-m would be more difficult as we'd have to cross compile and run in an emulator.
Comment Actions There are really two intended uses for the __sync_* builtins on ARM.
Neither of these is relevant for M-class processors. In other cases, the compiler should emit an inline sequence, or call one of the __atomic_* APIs. See http://llvm.org/docs/Atomics.html#libcalls-sync . If that isn't happening, it's a bug in clang/LLVM. The open question here, is whether it's appropriate to turn off interrupts in compiler-rt, if we're in a baremetal environment. I would lean towards no... if someone really wants to use an implementation that turns off interrupts, they can implement __atomic_* appropriately.
The instruction exists on A-class targets. It's just that on most A-class targets, we run in userspace, and userspace isn't allowed to disable interrupts, for obvious reasons. (Also, on A-class targets, we can't generally assume there's only one CPU core.) Comment Actions
M-profile also has two privilege levels, and the CPS instruction is not usable in unprivileged mode, so disabling interrupts won't always work (and the CPS instruction is a no-op in unprivileged mode, so we would silently get non-atomic accesses). Comment Actions Ok, based on the above comments I believe that this is a bug in LLVM and not a missing feature in compiler-rt. LLVM should generate __atomic_* calls instead of __sync_ calls and it shouldn't generate those for Cortex-M3 and up when doing 32-bit atomics because it should be able to emit such instructions inline. Unfortunately, I know very little about the backend to fix this myself. | ||||||||||||||||||||||||
Probably want to use https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html as this will track the latest release of GCC.