[PATCH] D61052: [compiler-rt][builtins] Implement some fetch-and-x operations for Cortex-M

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

In D61052#1483200 <https://reviews.llvm.org/D61052#1483200>, @efriedma wrote:

> Is there any reason to avoid 32-bit ldrex/strex on M-class processors other than Cortex-M0?


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.

> 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?

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.

> Is it possible that the value of primask might change after the mrs, but before the cpsid instruction?

In theory

- interrupts could be enabled at the point of the mrs instruction
- an interrupt could occur in between the mrs and the cpsid
- that interrupt could disable further interrupts and not re-enable them (unlikely) but possible.
- the cpsid will disable the interrupts (no-op as they are already disabled)
- the msr will re-enable interrupts

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?


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61052/new/

https://reviews.llvm.org/D61052