Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fdesbiens wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion ports/cortex_m0/gnu/src/tx_thread_context_restore.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,14 @@ _tx_thread_context_restore:
/* Call the ISR exit function to indicate an ISR is complete. */
PUSH {r0, lr} // Save return address
BL _tx_execution_isr_exit // Call the ISR exit function
POP {r0, lr} // Recover return address
POP {r0, r1} // Recover r0 and the return address
MOV lr, r1 // This core cannot POP into LR: the
// 16-bit Thumb POP takes r0-r7 and
// pc only. MOV to a high register
// is permitted, so restore LR from
// a scratch register. r1 is free --
// the BL above may clobber r0-r3,
// which is why r0 is saved at all.
#endif
BX lr
@}
9 changes: 8 additions & 1 deletion ports/cortex_m0/gnu/src/tx_thread_context_save.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,14 @@ _tx_thread_context_save:
/* Call the ISR enter function to indicate an ISR is starting. */
PUSH {r0, lr} // Save return address
BL _tx_execution_isr_enter // Call the ISR enter function
POP {r0, lr} // Recover return address
POP {r0, r1} // Recover r0 and the return address
MOV lr, r1 // This core cannot POP into LR: the
// 16-bit Thumb POP takes r0-r7 and
// pc only. MOV to a high register
// is permitted, so restore LR from
// a scratch register. r1 is free --
// the BL above may clobber r0-r3,
// which is why r0 is saved at all.
#endif

/* Context is already saved - just return. */
Expand Down
9 changes: 8 additions & 1 deletion ports/cortex_m23/gnu/src/tx_thread_context_restore.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,14 @@ _tx_thread_context_restore:
/* Call the ISR exit function to indicate an ISR is complete. */
PUSH {r0, lr} // Save return address
BL _tx_execution_isr_exit // Call the ISR exit function
POP {r0, lr} // Recover return address
POP {r0, r1} // Recover r0 and the return address
MOV lr, r1 // This core cannot POP into LR: the
// 16-bit Thumb POP takes r0-r7 and
// pc only. MOV to a high register
// is permitted, so restore LR from
// a scratch register. r1 is free --
// the BL above may clobber r0-r3,
// which is why r0 is saved at all.
#endif

BX lr
Expand Down
9 changes: 8 additions & 1 deletion ports/cortex_m23/gnu/src/tx_thread_context_save.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,14 @@ _tx_thread_context_save:
/* Call the ISR enter function to indicate an ISR is starting. */
PUSH {r0, lr} // Save return address
BL _tx_execution_isr_enter // Call the ISR enter function
POP {r0, lr} // Recover return address
POP {r0, r1} // Recover r0 and the return address
MOV lr, r1 // This core cannot POP into LR: the
// 16-bit Thumb POP takes r0-r7 and
// pc only. MOV to a high register
// is permitted, so restore LR from
// a scratch register. r1 is free --
// the BL above may clobber r0-r3,
// which is why r0 is saved at all.
#endif

BX lr
Expand Down
72 changes: 67 additions & 5 deletions scripts/check_clang.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 ""
Expand Down
Loading