mirror of
https://github.com/eclipse-threadx/threadx.git
synced 2026-10-06 06:59:08 +08:00
Made the basic processing counter visible to the reporting thread (#616)
The thread-metric basic processing test reports 0 for every period when built
with optimisation, and additionally claims "Basic processing thread died!"
because the counter never differs from the previous reading.
tm_basic_processing_counter is written by the processing thread and read by the
reporting thread, but it was a plain global. The processing loop calls nothing,
so nothing forces the compiler to write the counter back to memory, and in an
infinite loop there is no exit path where it must. At -O2 with
arm-none-eabi-gcc 13.2.1 the counter is loaded once before the loop, incremented
in a register, and never stored:
28: ldr ip, [r3] counter loaded once
... inner loop over the volatile array
50: add ip, ip, #1 increment stays in the register
54: b 2c and around again
The reporting thread reads the memory location, which stays 0 for the life of
the program. This is not specific to the Armv8-R target it was reported on: the
same shape appears for Cortex-M4 in Thumb state, and at -O1, -O2, -O3 and -Os.
Only -O0 happens to work.
Declare the counter volatile so the increment becomes a real store. Read it into
a local once per pass and use the local inside the 1024-iteration loop, rather
than letting every iteration re-read the volatile: that would add a memory access
to each iteration and change the amount of work the test performs. This test is
the baseline the rest of the suite is scaled against, per the readme, so its
throughput has to stay comparable with previously published figures and with
other RTOSes.
The array was already volatile, which is why the arithmetic itself survives
optimisation; only the counter was missing.
Verified by disassembly rather than by inspection. After the change the inner
loop is instruction-for-instruction identical to what dev generates today,
ldr/ldr/add/eor/str/add/cmp/bne, with one volatile read hoisted above the loop
and one store below it:
30: ldr ip, [lr] one read per pass, outside the loop
34: ... inner loop unchanged, back edge targets 34
54: add ip, ip, #1
58: str ip, [lr] the counter is now published
5c: b 2c
Confirmed for cortex-r52 in Arm state and cortex-m4 in Thumb state.
Reported by @hotislandn in #480, which identified the cause correctly.
Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
This commit is contained in:
@@ -41,7 +41,13 @@
|
||||
|
||||
/* Define the counters used in the demo application... */
|
||||
|
||||
unsigned long tm_basic_processing_counter;
|
||||
/* volatile because the reporting thread reads this while the processing thread
|
||||
below increments it. The processing loop calls nothing, so without volatile
|
||||
the compiler is free to keep the counter in a register for the lifetime of an
|
||||
infinite loop and never write it back: at -O2 it does exactly that, and the
|
||||
report reads 0 forever. */
|
||||
|
||||
volatile unsigned long tm_basic_processing_counter;
|
||||
|
||||
|
||||
/* Test array. We will just do a series of calculations on the
|
||||
@@ -98,7 +104,8 @@ void tm_basic_processing_initialize(void)
|
||||
void tm_basic_processing_thread_0_entry(void)
|
||||
{
|
||||
|
||||
int i;
|
||||
int i;
|
||||
unsigned long snapshot;
|
||||
|
||||
/* Initialize the test array. */
|
||||
for (i = 0; i < 1024; i++)
|
||||
@@ -111,6 +118,15 @@ int i;
|
||||
while(1)
|
||||
{
|
||||
|
||||
/* Read the counter once per pass and use the copy in the loop below.
|
||||
Reading the volatile counter inside the loop instead would add a
|
||||
memory access to every one of the 1024 iterations and so change the
|
||||
amount of work this test performs. That matters more here than
|
||||
elsewhere in the suite: this test is the baseline the other results
|
||||
are scaled against, so its throughput has to stay comparable with
|
||||
previously published figures and with other RTOSes. */
|
||||
snapshot = tm_basic_processing_counter;
|
||||
|
||||
/* Loop through the basic processing array, add the previous
|
||||
contents with the contents of the tm_basic_processing_counter
|
||||
and xor the result with the previous value... just to eat
|
||||
@@ -119,11 +135,11 @@ int i;
|
||||
{
|
||||
|
||||
/* Update each array entry. */
|
||||
tm_basic_processing_array[i] = (tm_basic_processing_array[i] + tm_basic_processing_counter) ^ tm_basic_processing_array[i];
|
||||
tm_basic_processing_array[i] = (tm_basic_processing_array[i] + snapshot) ^ tm_basic_processing_array[i];
|
||||
}
|
||||
|
||||
/* Increment the basic processing counter. */
|
||||
tm_basic_processing_counter++;
|
||||
tm_basic_processing_counter = snapshot + 1;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user