core/compiler_hints: add assume() hint

[benpicco]

Contribution description

This behaves like an assert() with DEVELHELP=1, but when DEVELHELP=0 it still gives the compiler a hint that the condition is guaranteed to be true and that it can optimize the code accordingly.

Testing procedure

Issues/PRs references

alternative to #19337

[chrysn]

LGTM.

Do you have any good example for testing where the use of assume() makes a difference, or do you plan to switch some initial hot candidates over in the original PR already?

[kfessel]

if we name this macro assume, what will a macro pointing to https://clang.llvm.org/docs/AttributeReference.html#id468 / https://en.cppreference.com/w/cpp/language/attributes/assume ,which do not check the assumption, be called?

maybe our assume is just safer in the debug case?

[chrysn]

If compilers provide any specialized form, it'd be possible to implement the (non-debug ... in debug mode it's probably helpful to check) assume function through their builtins. But outside corner cases, I doubt they'd make any difference: The one provided here is only evaluated if there's an external function or side effect invoked (why would one do that in an assume?), otherwise it may look like evaluating something, but by the outcome going into the unreachable, the compiler doesn't have to (and won't) emit code but just use the learned properties of the affected variables.

[riot-ci]

Murdock results

✔️ PASSED

a5bc1b3 core/compiler_hints: fix detection of __builtin_unreachable()

Success	Failures	Total	Runtime
6931	0	6931	10m:33s

Artifacts

• Documentation preview

[kfessel]

wouldn't the preprocessor rewrite [[assume(x > 0)]] to [[assert(x > 0)]]?

https://godbolt.org/z/xojhj9ab8
the assume macro is expanded and then the attribute syntax goes wrong

[chrysn]

Ah, thanks for the example. I find it highly weird (but at the same time very consistent) that these new attributes (are they really becoming a thing in C?) are not protected against preprocessor. But well. Is there any good reason why this can't be a static inline function in the first place? (assert can't be one, for it may not want to evaluate, but that's not an issue here for the assumed expression is always "evaluated").

[maribu]

+1 for giving static inline a try.

IMO we should also provide a semantic patch to let the CI insist on assume() being used over assert(). Every quarter or so @gschorcht fixes compilation with NDEBUG and it is a a few dozen instances of function arguments not being used (well, they are used by assert() without NDEBUG) every time. That issue would be solved once and for all with switching to assume(). In addition it would be crystal clear that error handling with assume() is not to be done and hopefully everyone doea proper error handling (where recovery is possible and sensible) or uses expect() (where bringing thr chip into a stopped state is the most sensible thing to do).

[benpicco]

The reason why a define is nicer is that it doesn't care about types.

If I have a static inline void assume(bool cond) that won't work with assume(ptr) - OK, maybe it would be better to write assme(ptr != NULL) , but I see several assert(ptr) so this is not something very unusual.

But this is not a mayor thing, if you really prefer the static inline function over the define, I'm fine with this slight inconvenience.

[maribu]

OK, let's have a define. It would be good to have this as a drop-in replacement as much as possible.

[maribu]

bors merge

[bors]

Build succeeded:

• Murdock
• static-tests
• tools-build-success