Skip to content

atomic.h: ARM guards are broken for mi_atomic_yield #1046

@vfazio

Description

@vfazio

PR #339 added an ARM version check (__ARM_ARCH__ >= 7) to guard __asm__ volatile("yield" ::: "memory");

However, GCC prefers to use __ARM_ARCH per the ACLE https://arm-software.github.io/acle/main/acle.html#mapping-of-object-build-attributes-to-predefines

armeb-buildroot-linux-gnueabi-gcc -dM -E - < /dev/null | grep -i arm
#define __ARM_SIZEOF_WCHAR_T 4
#define __ARM_FEATURE_SAT 1
#define __ARM_ARCH_ISA_ARM 1
#define __ARM_FEATURE_IDIV 1
#define __ARM_SIZEOF_MINIMAL_ENUM 4
#define __ARMEB__ 1
#define __ARM_FEATURE_LDREX 15
#define __ARM_PCS 1
#define __ARM_FEATURE_QBIT 1
#define __ARM_ARCH_PROFILE 65
#define __ARM_32BIT_STATE 1
#define __ARM_FEATURE_CLZ 1
#define __ARM_ARCH_ISA_THUMB 2
#define __ARM_ARCH 7
#define __ARM_BIG_ENDIAN 1
#define __ARM_FEATURE_UNALIGNED 1
#define __arm__ 1
#define __ARM_ARCH_7A__ 1
#define __ARM_FEATURE_SIMD32 1
#define __ARM_FEATURE_COPROC 15
#define __ARM_FEATURE_DSP 1
#define __ARM_ARCH_EXT_IDIV__ 1
#define __ARM_EABI__ 1

This wasn't a huge issue because it would fall back to the following implementation (I think):

static inline void mi_atomic_yield(void) {
  sleep(0);
}

However, since PR #350 the mi_atomic_yield function may be undefined or use the incorrect instruction during some builds:

On ARM builds:

  1. __arm__ is likely always defined so we enter into the #ifdef
  2. if __ARM_ARCH__ is not defined, which is probably standard on most GCCs, and it's a little-endian build, it's caught by defined(__armel__) || defined(__ARMEL__) and has a body emitted, though if the architecture is v7+ it will not use yield as probably intended.
  3. If it's a big-endian architecture, no function body is emitted and the build fails.

I'm not sure there needs to be a distinction between EL and EB builds, but i'll leave that to the experts. The define can maybe be something like:

#elif defined(__arm__)
#if __ARM_ARCH >= 7
static inline void mi_atomic_yield(void) {
  __asm__ volatile("yield" ::: "memory");
#else
  asm volatile ("nop" ::: "memory");
#endif

I don't know enough about the arm32 to say that nop ::: memory should be used over sched_yield or sleep 0.

Regardless, as-is, big endian ARM builds fail and LE builds are likely using nop even when yield is supported. This is currently affecting CPython 3.13 builds for Buildroot (https://autobuild.buildroot.org/results/26b752738022e8b46e810a08e28d687120e5c4e3/build-end.log)

I can patch our build process, but felt like I should report it here. I will likely create an issue in CPython some time tomorrow and link to this issue for the fix to be backported.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions