From acdc02b2fcd5883faed61a619d38f8e5f89fc69e Mon Sep 17 00:00:00 2001 From: Frédéric Desbiens Date: Thu, 13 Aug 2026 10:14:53 -0400 Subject: Assembled the code behind feature macros, and fixed the POP it found (#608) 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) --- scripts/check_clang.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 5 deletions(-) (limited to 'scripts') diff --git a/scripts/check_clang.sh b/scripts/check_clang.sh index c0cae401..c7ebed79 100755 --- a/scripts/check_clang.sh +++ b/scripts/check_clang.sh @@ -14,11 +14,11 @@ # SPDX-License-Identifier: MIT and CC0-1.0 ############################################################################## -# Builds the Arm ports with an LLVM based toolchain, in four stages: assemble -# every assembly source of every Arm gnu port, compile the common C sources for -# one core per architecture profile, link the example builds that have a script -# driver, then link those driven by CMake. Only the two linking stages need a -# target C library. +# Builds the Arm ports with an LLVM based toolchain, in five stages: assemble +# every assembly source of every Arm gnu port, assemble again the parts guarded +# by feature macros, compile the common C sources for one core per architecture +# profile, then link the example builds, both the script-driven ones and those +# driven by CMake. Only the linking stages need a target C library. # # scripts/check_clang.sh # clang from PATH # scripts/check_clang.sh --clang /path/to/clang @@ -140,6 +140,28 @@ declare -A PORT_TARGET=( [cortex_a78_smp]="aarch64-none-elf cortex-a78" ) +# Assembly guarded by a feature macro is invisible to the stage above, which +# assembles with default flags and so lets the preprocessor discard every #ifdef +# block before the assembler sees it. These are the macros a user can turn on; +# each file carrying one is assembled again with it defined. +# +# This is not hypothetical. It is where "POP {r0, lr}" was found in the Cortex-M0 +# and Cortex-M23 execution-profile paths: invalid on Armv6-M and Armv8-M +# Baseline, where the 16-bit POP takes r0-r7 and pc only, and rejected by GNU as +# well as by LLVM. Turning the feature on had never once been tried. +FEATURE_MACROS="TX_ENABLE_VFP_SUPPORT TX_LOW_POWER TX_ENABLE_EXECUTION_CHANGE_NOTIFY" + +# Extra flags for the VFP paths, per core, needed only where -mcpu alone cannot +# assemble them. Cortex-R4's FPU is an option rather than part of the core, so +# both toolchains reject its VFP code without an -mfpu. +# +# Do not extend this to the A profile ports. They save D16-D31, which exists only +# on a 32-register FPU, so naming a -d16 FPU takes those registers away and turns +# 28 working files into "register expected". Their defaults are already correct. +declare -A VFP_EXTRA=( + [cortex_r4]="-mfpu=vfpv3-d16 -mfloat-abi=softfp" +) + # One core per architecture profile for the C sources. Compiling all of them # for every core would multiply the run time without adding coverage, since the # port headers differ by profile rather than by core. @@ -200,6 +222,46 @@ if [ -n "$skipped" ]; then say " not Arm, skipped:$(echo $skipped | tr ' ' '\n' | sort -u | tr '\n' ' ')" fi +# -------------------------------------------------------------------------- +say "" +say "== Assembly behind feature macros ==" + +for macro in $FEATURE_MACROS; do + macro_total=0 + macro_bad=0 + for src in $(grep -rl "$macro" ports/*/gnu/src/*.S ports_smp/*/gnu/src/*.S \ + 2>/dev/null | sort); do + core="$(echo "$src" | cut -d/ -f2)" + spec="${PORT_TARGET[$core]:-}" + [ -n "$spec" ] || continue + # shellcheck disable=SC2086 + set -- $spec + target="$1"; cpu="$2"; shift 2; extra="$*" + + # The FPU flags apply to the VFP paths only; the other macros guard no + # floating-point code and do not need them. + fpu="" + if [ "$macro" = "TX_ENABLE_VFP_SUPPORT" ]; then + fpu="${VFP_EXTRA[$core]:-}" + fi + + macro_total=$((macro_total + 1)) + output="$("$CC" --target="$target" -mcpu="$cpu" $extra $fpu \ + -D"$macro" -c "$src" -o /dev/null 2>&1)" + if [ -n "$output" ]; then + fail "$src with -D$macro" + echo "$output" | grep "error:" | head -3 | sed 's/^/ /' + macro_bad=$((macro_bad + 1)) + failures=$((failures + 1)) + fi + done + if [ "$macro_total" -eq 0 ]; then + say " $macro: no assembly is guarded by it" + else + say " $macro: $((macro_total - macro_bad)) of $macro_total assembled" + fi +done + # -------------------------------------------------------------------------- if [ "$asm_only" -eq 0 ]; then say "" -- cgit v1.3.1