build system: add float_math feature to indicate support of floating point arithmetic #21727
Conversation
maribu
added
the
CI: ready for build
github-actions
bot
added
Platform: native
crasbe
added
Type: bug
maribu
force-pushed
the
features-float
branch
from
2b4f0e7 to
5826484
Compare
This adds the float_math feature to express that math on floating point numbers is supported. All CPUs but native provide this, to reflect that apps using floats on native will randomly fail See RIOT-OS#495 Co-authored-by: crasbe <[email protected]>
In some modules some functions are disabled when no floating point arithmetics is available instead of blocking the whole module. The API doc has been extended, so that users know why a function may fail to link. Co-authored-by: crasbe <[email protected]>
We just use a simple look-up table for the 10^x exponents that do not overflow the range of int16_t. In addition, we saturate on overflows.
When the `float_math` feature is not in used, this adds CFLAGS to instruct the compiler to not make use of FPU/SSE registers, fixing issues (random crashes, floating point exceptions, stack corruptions, incorrect computations) due to incorrect back and restore of those registers on context switching. If `float_math` is used, the buggy behavior as before is produced. But since `float_math` is not actually provided by the native CPU, one would have to compile `CONTINUE_ON_EXPECTED_ERRORS=1` to actually get into that situation. Co-authored-by: Mihai Renea <[email protected]>
maribu
force-pushed
the
features-float
branch
from
3a5ac43 to
56be5ab
Compare
github-actions
bot
added
Area: pkg
|
Wouldn't it make more sense to add IMO it's bad enough that |
| ifneq (,$(filter printf_float,$(USEMODULE))) | ||
| FEATURES_REQUIRED += float_math | ||
| endif |
There was a problem hiding this comment.
That is actually done here. But most apps that do print float/double also do math with float/double before printing it.
It would be possible to actually implement the %f format specifier in printf without using floating point arithmetic directly. But then those apps would still not work on systems that do not provide floating point arithmetic if the use float math themselves.
| LINKFLAGS += $(shell pkg-config libucontext --libs) | ||
| endif | ||
|
|
||
| ifeq (x86_64,$(OS_ARCH)) |
There was a problem hiding this comment.
Don't we also need that on 32 bit?
There was a problem hiding this comment.
Yes, but I wonder if someone actually still has an i585/i686 CPU in use these days.
It won't hurt to add the check, though.
There was a problem hiding this comment.
It's more about testing the code on a 32 bit target which is what most of our targets are.
There was a problem hiding this comment.
But the output of uname -m would still be x86_64, even with BOARD=native32.
But I honestly don't really see a path forward here with including C++ headers resulting in compilation errors when SSE is disabled.
There was a problem hiding this comment.
Ah I thought OS_ARCH was referring to RIOT 😅
4 participants
Contribution description
float_mathfeature to indicate support of floating point arithmeticnativeprovide thisFEATURES_REQUIRED += float_mathto all drivers/modules that use floating point arithmeticnativewhen running on x86_64: Disable use of FPU/SSE registers on native unlessfloat_mathis in usedCONTINUE_ON_EXPECTED_ERRORS=1it is possible to usefloat_mathonnative. But then we are back to having random crashes / incorrect computations / stack corruptions / floating point exceptions on context switchingTesting procedure
native, nothing should changenativeno longer need that, either because they use floating point arithmetic (and now automatically won't be build) or should work nowIssues/PRs references
Fixes #495 by properly modeling this in the build system. But more importantly: It fixes issues where the compiler spilled values onto SSE/FPU registers without the use of floating point arithmetic (e.g. seen in the wild in nanopb).