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:
- 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.
- Spell it
__asm__, not asm. Under -std=c99 the asm keyword is not recognised
(error: 'asm' undeclared).
- 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.
Affects: ~24
tx_port.hfiles acrossports/,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.elfregression image. Referenced by subject rather than by SHA because that branch is rebased ondevperiodically and a bare hash goes stale.What is wrong
Every AArch32
tx_port.hoverridesTX_LOWEST_SET_BIT_CALCULATEwith a CLZimplementation behind this guard:
__TARGET_ARCH_ARMis an Arm Compiler 5 (armcc) predefine. GCC does not define it —it predefines the ACLE macros
__ARM_ARCHand__ARM_FEATURE_CLZ. armclang (ArmCompiler 6) is Clang-based and does the same. So under either compiler the directive reads
0 > 4, the block is dropped, andtx_thread.h's portable loop runs on cores that havehad the instruction since Armv5.
Because the identifier is undefined rather than zero, the preprocessor is silent about it.
-Wundefreports it; upstream's own kernel warning set intest/tx/cmake/CMakeLists.txtdoes not include that flag, which is why it has stayedquiet.
Confirmation
No
__TARGET_ARCH_ARM. And in the built library (arm-none-eabi-gcc 14.3, Cortex-R52,default options),
arm-none-eabi-objdump -dfinds zero CLZ instructions anywhere inthe 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:
> 32common/src/tx_thread_suspend.ccommon/src/tx_thread_system_suspend.cSo 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 defaultconfiguration — same compiler, same flags, only the guard changed:
.text40% smaller, on a hot path.
Suggested fix
__ARM_FEATURE_CLZis the ACLE answer to the question actually being asked, so a corewithout 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 relieson an undefined identifier.
Three details that are easy to get wrong:
#ifndef __thumb__.__ARM_FEATURE_CLZdescribes the architecture, not theinstruction set — GCC defines it for
-mthumb -march=armv5te, where Thumb-1 has no CLZand the asm will not assemble. Removing that guard breaks the older ports' Thumb builds.
__asm__, notasm. Under-std=c99theasmkeyword is not recognised(
error: 'asm' undeclared).(ULONG) (-((LONG) m))converts an unsigned map tosigned and negates it, which is undefined for
m == 0x80000000.(~(m)) + 1is the samevalue in well-defined unsigned arithmetic, and is what
tx_thread.h's portable versionalready uses.
One behavioural difference worth documenting
The two implementations disagree for
m == 0:CLZ(0)is 32, so the CLZ version yields31 - 32, while the portable loop yields 0. All twelve call sites incommon/srcreach themacro 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
gnuport, everyac6port,ports_arch/ARMv7-A,ports_module/, and theports_smp/*/gnuports.ports/arm9/ac5is 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, bothUINTandULONGresults, and them == 0case — and which fails to build if the CLZ pathis 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.