Arduino
Arduino copied to clipboard
Prevent abs() macro from sign-flipping 0.0F
Fixes #8115
This variant solves the abs(-0.0F) := -0.0F
problem, at the expense of a few CPU cycles but also 32 bytes code:
#define abstest(x) ({ __typeof__(x) _x = (x); _x >= 0 ? _x + 0 : -_x; })
In the original PR in the AVR, I fail to see the actual bug they're solving. If -0.0f == 0.0f
then why would one care?
Since this is a macro (thanks, Arduino) it seems like it will add 32 bytes per call, everywhere, with the change, which seems large.
@earlephilhower Within a single compilation unit, the compiler seems to optimize that overhead. But as you can tell from me not putting it into the PR, instead just giving it a showing in a remark, I don't see much need for that myself. As for this PR, I find some merit in having all-zero bits representation at least for the positive zero, such that (uint32_t)0 and (float)0 are the same in memory. For what it's worth :-)
There's also another approach - just use compiler built-ins to implement overloading, as floats also include things like inf
and nan
.
https://gist.github.com/mcspr/8b9a317b1cbe27085c63a1e7a885902e
(and pretty much re-implement std::abs, so c macro matches the current c++, or leave the long
as-is and only override floats)
Not sure of code generation though, but it seems more of actually solving the issue instead of working around a certain part. ref. https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr and https://clang.llvm.org/docs/LanguageExtensions.html#builtin-functions
@mcspr This is not as much about a more perfect abs() function, as it is about being Arduino-alike.
@mcspr This is not as much about a more perfect ab() function, as it is about being Arduino-alike.
then, I'd agree with the comment above - no point in fixing anything, as neither abs or round macros are provided in the current ArduinoCore API headers in favour of math.h: https://github.com/arduino/ArduinoCore-API/issues/76#issuecomment-781132117
and current macro can stay as-is, until the issue above is definitely closed and there's a clear answer what arduino side want to do with them