Skip to content

Assembled the code behind feature macros, and fixed the POP it found - #608

Merged
fdesbiens merged 1 commit into
eclipse-threadx:devfrom
fdesbiens:feature/llvm-check-feature-macros
Aug 13, 2026
Merged

Assembled the code behind feature macros, and fixed the POP it found#608
fdesbiens merged 1 commit into
eclipse-threadx:devfrom
fdesbiens:feature/llvm-check-feature-macros

Conversation

@fdesbiens

Copy link
Copy Markdown
Contributor

scripts/check_clang.sh assembled every source with default flags, so the
preprocessor discarded each #ifdef block before the assembler saw it. No
guarded path in the tree had ever been assembled
— that covers the VFP context
save and restore in ten ports, plus 218 files carrying TX_LOW_POWER or
TX_ENABLE_EXECUTION_CHANGE_NOTIFY.

Turning them on found a defect.

The defect

The Cortex-M0 and Cortex-M23 execution-profile paths bracket their call like this:

    PUSH    {r0, lr}
    BL      _tx_execution_isr_enter
    POP     {r0, lr}

That last instruction is invalid on Armv6-M and Armv8-M Baseline, where the 16-bit
Thumb POP takes r0-r7 and pc and nothing else. GNU rejects it too
"cannot honor width suffix" — so TX_ENABLE_EXECUTION_CHANGE_NOTIFY and
TX_EXECUTION_PROFILE_ENABLE have never been buildable on either port, with
either toolchain. Four files, all the same shape.

The fix pops into a scratch register and moves it, MOV to a high register being
permitted where POP is not. r1 is free — the BL may clobber r0-r3, which is
precisely why r0 is saved. Disassembled:

   0:	b501      	push	{r0, lr}
   2:	f7ff fffe 	bl	0 <_tx_execution_isr_enter>
   6:	bc03      	pop	{r0, r1}
   8:	468e      	mov	lr, r1
   a:	4770      	bx	lr

One 16-bit instruction more than before, otherwise identical.

Two findings that were not defects

Both are recorded in the script, because both cost me a detour:

Cortex-R4 needs an -mfpu to assemble its VFP path. Its FPU is an option
rather than part of the core. GNU fails identically without one, so this is a
flags requirement, not a toolchain divergence.

The A profile ports must not be given one. Adding -mfpu=vfpv3-d16 uniformly
broke 28 files with "register expected", because those ports save D16-D31 and a
-d16 FPU does not have those registers. Their defaults were already correct. The
comment in the script says so, so the next person does not repeat it.

The new stage

Each guarded file is assembled a second time with its macro defined. It runs under
--asm-only as well, since it needs no target C library.

== Assembly behind feature macros ==
  TX_ENABLE_VFP_SUPPORT: 37 of 37 assembled
  TX_LOW_POWER: 8 of 8 assembled
  TX_ENABLE_EXECUTION_CHANGE_NOTIFY: 218 of 218 assembled

Verified non-vacuous: restoring the POP for one run makes the stage exit 1
with 217 of 218, naming the file and the error.

The other four stages are unchanged — 711 of 711 assembly sources, 185 of 185
common C sources for each of nine cores, 42 of 42 script-driven examples, 5 of 5
CMake images — and the whole check passes.

What is not verified

The fixed code assembles with both toolchains and encodes as intended. It is
not verified running: there is no Cortex-M0 or Cortex-M23 model available
here, and these are context save and restore paths. Worth a careful review of the
four hunks on that basis, or a run on real hardware by anyone who has either part
to hand.

scripts/check_clang.sh assembled every source with default flags, so the
preprocessor discarded each #ifdef block before the assembler saw it. Nothing in
the tree had ever assembled a guarded path. That covers the VFP context save and
restore in ten ports, and 218 files carrying TX_LOW_POWER or
TX_ENABLE_EXECUTION_CHANGE_NOTIFY.

Turning those on found a defect. The Cortex-M0 and Cortex-M23 execution-profile
paths bracket their call with

    PUSH    {r0, lr}
    BL      _tx_execution_isr_enter
    POP     {r0, lr}

and the last of those is invalid on Armv6-M and Armv8-M Baseline, where the
16-bit Thumb POP takes r0-r7 and pc and nothing else. GNU rejects it as well --
"cannot honor width suffix" -- so TX_ENABLE_EXECUTION_CHANGE_NOTIFY and
TX_EXECUTION_PROFILE_ENABLE have never been buildable on either port with either
toolchain. Four files, all the same shape.

The fix pops into a scratch register and moves it, MOV to a high register being
permitted where POP is not. r1 is free: the BL may clobber r0-r3, which is the
reason r0 is saved in the first place. Disassembling the result gives
push {r0, lr} / bl / pop {r0, r1} / mov lr, r1 / bx lr, one 16-bit instruction
more than before and otherwise the same.

Two findings that were not defects, recorded in the script so they are not
rediscovered:

Cortex-R4 needs an -mfpu to assemble its VFP path, because its FPU is an option
rather than part of the core. GNU fails identically without one, so this is a
flags requirement and not a toolchain divergence.

The A profile ports must not be given one. Adding -mfpu=vfpv3-d16 uniformly broke
28 files with "register expected", because those ports save D16-D31 and a -d16
FPU does not have those registers. Their defaults were already right.

The new stage runs under --asm-only as well, needing no target C library, and
reports 37 of 37 VFP files, 8 of 8 TX_LOW_POWER and 218 of 218
TX_ENABLE_EXECUTION_CHANGE_NOTIFY. Restoring the POP for one run makes it fail
with 217 of 218 and name the file and the error, so the stage is not vacuous.
The other four stages are unchanged: 711 of 711 assembled, 185 of 185 common C
sources for each of nine cores, 42 of 42 script-driven examples and 5 of 5 CMake
images.

The fixed code is verified to assemble with both toolchains and to encode as
intended. It is not verified running: there is no Cortex-M0 or Cortex-M23 model
here, and these are context save and restore paths, so that gap is worth stating.

Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
@Lvbor

Lvbor commented Aug 13, 2026

Copy link
Copy Markdown

Hello @fdesbiens, I've successfully tested this patch and confirmed it to be working on a Cortex M23.

@fdesbiens

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback, @Lvbor! Merging this to dev now.

@fdesbiens
fdesbiens merged commit acdc02b into eclipse-threadx:dev Aug 13, 2026
4 checks passed
@fdesbiens
fdesbiens deleted the feature/llvm-check-feature-macros branch August 13, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants