Skip to content

The CLZ lowest-set-bit optimisation has never run under GCC or armclang #650

Description

@fdesbiens

Affects: ~24 tx_port.h files across ports/, ports_arch/, ports_module/, ports_smp/
Type: performance / code size, all AArch32 ports
Working patch: the commit "Woke the CLZ priority search, which had never once run under GCC" on the branch behind #639, which fixes the two Cortex-R52 ports and adds the demo_clz.elf regression image. Referenced by subject rather than by SHA because that branch is rebased on dev periodically and a bare hash goes stale.

What is wrong

Every AArch32 tx_port.h overrides TX_LOWEST_SET_BIT_CALCULATE with a CLZ
implementation behind this guard:

#if __TARGET_ARCH_ARM > 4

__TARGET_ARCH_ARM is an Arm Compiler 5 (armcc) predefine. GCC does not define it —
it predefines the ACLE macros __ARM_ARCH and __ARM_FEATURE_CLZ. armclang (Arm
Compiler 6) is Clang-based and does the same. So under either compiler the directive reads
0 > 4, the block is dropped, and tx_thread.h's portable loop runs on cores that have
had the instruction since Armv5.

Because the identifier is undefined rather than zero, the preprocessor is silent about it.
-Wundef reports it; upstream's own kernel warning set in
test/tx/cmake/CMakeLists.txt does not include that flag, which is why it has stayed
quiet.

Confirmation

$ arm-none-eabi-gcc -marm -mcpu=cortex-r52 -dM -E - < /dev/null | grep -E 'TARGET_ARCH|__ARM_FEATURE_CLZ|__ARM_ARCH '
#define __ARM_FEATURE_CLZ 1
#define __ARM_ARCH 8

No __TARGET_ARCH_ARM. And in the built library (arm-none-eabi-gcc 14.3, Cortex-R52,
default options), arm-none-eabi-objdump -d finds zero CLZ instructions anywhere in
the scheduler objects.

Why this is not a dormant path

It would be easy to assume this only matters for TX_MAX_PRIORITIES > 32. It does not.
Half the call sites are outside those guards:

File Call sites gated on > 32 Call sites always compiled
common/src/tx_thread_suspend.c 442, 567 514, 581
common/src/tx_thread_system_suspend.c 311, 435, 900, 1010 385, 449, 969, 1024

So the portable loop has been running the scheduler's priority search in the default
32-priority configuration
, on every suspend and resume, on every AArch32 GNU and
armclang port.

Cost

Measured on tx_thread_system_suspend.o, the object that carries the macro in the default
configuration — same compiler, same flags, only the guard changed:

.text instructions
portable loop (today) 2188 bytes 547
CLZ (guard fixed) 1316 bytes 329

40% smaller, on a hot path.

Suggested fix

#if defined(__ARM_FEATURE_CLZ) || (defined(__TARGET_ARCH_ARM) && (__TARGET_ARCH_ARM > 4))
#ifndef __thumb__
#define TX_LOWEST_SET_BIT_CALCULATE(m, b)                                       \
    (m) =  (m) & ((~(m)) + ((ULONG) 1));                                        \
    __asm__ volatile (" CLZ  %0,%1 " : "=r" (b) : "r" (m));                     \
    (b) =  31 - (b);
#endif
#endif

__ARM_FEATURE_CLZ is the ACLE answer to the question actually being asked, so a core
without the instruction is excluded by construction rather than by an architecture number.
Arm Compiler 5's spelling is kept, now inside defined() so the directive no longer relies
on an undefined identifier.

Three details that are easy to get wrong:

  1. Keep #ifndef __thumb__. __ARM_FEATURE_CLZ describes the architecture, not the
    instruction set — GCC defines it for -mthumb -march=armv5te, where Thumb-1 has no CLZ
    and the asm will not assemble. Removing that guard breaks the older ports' Thumb builds.
  2. Spell it __asm__, not asm. Under -std=c99 the asm keyword is not recognised
    (error: 'asm' undeclared).
  3. The isolation step. Upstream's (ULONG) (-((LONG) m)) converts an unsigned map to
    signed and negates it, which is undefined for m == 0x80000000. (~(m)) + 1 is the same
    value in well-defined unsigned arithmetic, and is what tx_thread.h's portable version
    already uses.

One behavioural difference worth documenting

The two implementations disagree for m == 0: CLZ(0) is 32, so the CLZ version yields
31 - 32, while the portable loop yields 0. All twelve call sites in common/src reach the
macro only on a map already tested against zero, so this is unreachable today — but it is
undocumented, and a future call site would step on it silently. Worth a comment on the
macro either way.

Scope

Affected: every gnu port, every ac6 port, ports_arch/ARMv7-A, ports_module/, and the
ports_smp/*/gnu ports. ports/arm9/ac5 is genuinely unaffected, since armcc does define
__TARGET_ARCH_ARM.

Happy to raise a PR. The linked commit fixes the two Cortex-R52 ports and adds a regression
image (demo_clz.elf) that checks all 32 bit positions, 32 lowest-of-many patterns, both
UINT and ULONG results, and the m == 0 case — and which fails to build if the CLZ path
is ever disabled again, so it cannot silently go back to testing the portable loop. The
other ports were left alone deliberately: there is no build for them here and I would rather
not ship changes I cannot run.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions