core/thread: Allow for inline thread_yield_higher

[bergzand]

Contribution description

This PR modifies core/include/thread.h to allow for inlining the thread_yield_higher function similar to the irq api. Patches for the relevant CPUs have been included to either add the thread_arch.h header with the inlined function or as dummy header.

Initial benchmarks show that this shaves of 13 cycles from the bench_thread_yield_pingpong test on the nrf52dk board.

TODO: add headers for arm7, esp8266 and msp430.

Testing procedure

• Benchmarks: TODO
• The threading tests must still work on all affected platforms.

I can run the tests for the cortex-m and the RISC-V platform myself, but I could use some help with the mips32r2 (pic32-wifire or similar) platform.

Benchmarks

Comparing flash size and "ticks" parameter of tests/bench_thread_yield_pingpong:

board	flash pre	flash post	bench pre	bench post
samr21-xpro	11760	11836	575	546
nrf52dk	10812	10892	414	399
msba2	19924	19948	715	697
hifive1b	11108	11148	468	460

Issues/PRs references

None

[maribu]

Funny, I had just this also on my wish list. Thanks for tackling this :-)

(I wonder if we will undo this and the other inline headers again once LTO=1 becomes default 🤣 - but that should be easier.)

[bergzand]

(I wonder if we will undo this and the other inline headers again once LTO=1 becomes default 🤣 - but that should be easier.)

Probably, but as you say, that should be relative low effort

[maribu]

Would be nice to see benchmarks for at least one board per arch, so that this is motivated.

[bergzand]

First for the cortex-m0+ and the cortex-m4 case using bench_thread_yield_idle, using the ticks result:

board	flash pre	flash post	bench pre	bench post
samr21-xpro	11760	11836	575	546
nrf52dk	10812	10892	414	399

[maribu]

I wonder why this increases ROM noticeably. I smell another missed optimization opportunity in GCC.

[bergzand]

I wonder why this increases ROM noticeably. I smell another missed optimization opportunity in GCC.

On the nrf52dk, the sequence is LDR, MOV.W, STR and ISB. The LDR and STR are 16 bit, the other two 32 bit. A 32 bit word is used at the end of the function to store the SCB->ICSR register memory address in. 16 bytes in total for the whole sequence, every time the thread_yield_higher is used.

[maribu]

We're only missing test / benchmark for the fe310 and MIPS. And the ESP platform has no pseudo header yet.

@aabadie: Would you mind giving tests/bench_thread_yield_pingpong a spin on your Hifive 1?

@francois-berder: Would you mind to do the same on one of your MIPS boards?

[bergzand]

And the ESP platform has no pseudo header yet.

Here it is

[bergzand]

And the hifive1b (fe310) case using bench_thread_yield_idle, using the ticks result:

board	flash pre	flash post	bench pre	bench post
hifive1b	11108	11148	468	460

Trading 40 bytes for 8 clock ticks :)

[maribu]

Add for now only and empty pseudo-header for MIPS to get this in swiftly?

[benpicco]

This only moves an existing function into a header, do we really expect any change in behavior because of that?

[bergzand]

This only moves an existing function into a header, do we really expect any change in behavior because of that?

The function is inlined, influencing the performance, so yeah, I would prefer to have hard numbers before changing this.

In terms of flash I see a 20B increase on the pick32-wifire and a 24B increase on the 6lowpan-clicker.

[benpicco]

native needs an update too.

[bergzand]

native needs an update too.

Of course 😑, Added

[benpicco]

looks good to me.

[maribu]

I'd say we don't need to wait for MIPS results. Every architecture without exception got a speed bump im a hot code path by this, as was the expectation. Odds are good that for MIPS theory and practice also match.

[bergzand]

Thanks!