Skip to content

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Jun 11, 2021

Fixes #8115

@dok-net
Copy link
Contributor Author

dok-net commented Jun 11, 2021

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; })

@earlephilhower
Copy link
Collaborator

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.

@dok-net
Copy link
Contributor Author

dok-net commented Jun 12, 2021

@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 :-)

@mcspr
Copy link
Collaborator

mcspr commented Oct 9, 2021

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

@dok-net
Copy link
Contributor Author

dok-net commented Oct 9, 2021

@mcspr This is not as much about a more perfect abs() function, as it is about being Arduino-alike.

@mcspr
Copy link
Collaborator

mcspr commented Oct 9, 2021

@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:
arduino/ArduinoCore-API#76 (comment)

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

@dok-net dok-net force-pushed the fix_8115 branch 2 times, most recently from 3c084a7 to 13e62b8 Compare October 16, 2021 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

abs() macro in Arduino.h inverts signedness of (positive) 0.0F

3 participants