search

arduino / ArduinoCore-avr

The Official Arduino AVR core
https://www.arduino.cc
1.25k stars 1.06k forks source link

Update math macros to avoid computing callable arguments more than once #391

Closed Keating950 closed 3 years ago

Keating950 commented 3 years ago

Currently, if you pass an a callable argument to (e.g.) the max macro, the argument will be computed more than once. max(expensive_function(a), expensive_function(b)) will expand to:

expensive_function(a) > expensive_function(b) ? expensive_function(a) : expensive_function(b)

The compiler can't always optimize these calls away, as seen in the Godbolt disassembly here. This PR uses the GCC typeof extension to avoid these unnecessary calls where possible, falling back to the existing macros should that feature be unavailable. (Testing for it with the latest Arduino SDK shows that the extension is available.)

matthijskooijman commented 3 years ago

Note that min/max have already been updated similarly to what you do in this PR (and with a template function for C++) in the https://github.com/arduino/ArduinoCore-API repository. That repo will be used as a basis for the common code in this repo in the (near?) future, so that would be the best place to make fixes like these (I don't think changes to the common code will be merged here anymore).

As I said, just min and max have been fixed in that repo, the others are still up for improvement. See https://github.com/arduino/ArduinoCore-API/issues/85 for discussion about this, and a partial implementation that could be used as a basis. @Keating950, if you're interested in this, maybe you could make a PR over there? The -API repo is already used by the megaavr and SAMD boards, so it's easy to test there, and I think there's also a branch in this repo that used the -API repo (but it might be a bit old).

Keating950 commented 3 years ago

That sounds like the best place to start. Thanks for the direction!