Should I use if unlikely for hard crashing errors?

Question

I often find myself writing a code that looks something like this:

if(a == nullptr) throw std::runtime_error("error at " __FILE__ ":" S__LINE__);

Should I prefer handling errors with if unlikely?

if unlikely(a == nullptr) throw std::runtime_error("error at " __FILE__ ":" S__LINE__);

Will the compiler automatically deduce which part of the code should be cached or is this an actually useful thing to do? Why do I not see many people handling errors like this?

Elijah

What actually do you think unlikely() is for, if not for exactly this? – Yean

This question is lacking context. I asked my friend DuckDuckGo so that others don't have to: likely/unlikely are Linux kernel macros that wrap GCC's __builtin_expect. In other words, the question is specific to Linux kernel programming, or to those willing to #define their own non-portable macros of the same name. – Grossman

Also, as such, the c++ and c++1z tags seem inappropriate because C++ is not supported in Linux [kernel] programming, which is written mostly in C and inline assembler. (Although there is at least one C++-enabled fork.) – Grossman

FWIW, this is what Cython does in its code generation. – Ravine

Without indications, gcc already guesses that a is probably not 0 (around 80%) and that a branch with throw is very unlikely (less than 1%). An explicit annotation does not add much to this. You can pass -fdump-tree-optimized to see the estimated probabilities in a dump file. – Diacritical

Answer 1

Should I use "if unlikely" for hard crashing errors?

For cases like that I'd prefer moving code that throws to a standalone extern function that's marked as noreturn. This way, your actual code isn't "polluted" with lots of exception-related code (or whatever your "hard crashing" code). Contrary to the accepted answer, you don't need to mark it as cold, but you really need noreturn to make compiler not to try generating code to preserve registers or whatever state and essentially assume that after going there there is no way back.

For example, if you write code this way:

#include <stdexcept>

#define _STR(x) #x
#define STR(x) _STR(x)

void test(const char* a)
{
    if(a == nullptr)
        throw std::runtime_error("error at " __FILE__ ":" STR(__LINE__));
}

compiler will generate lots of instructions that deal with constructing and throwing this exception. You also introduce dependency on std::runtime_error. Check out how generated code will look like if you have just three checks like that in your test function:

First improvement: to move it to a standalone function:

void my_runtime_error(const char* message);

#define _STR(x) #x
#define STR(x) _STR(x)

void test(const char* a)
{
    if (a == nullptr)
        my_runtime_error("error at " __FILE__ ":" STR(__LINE__));
}

this way you avoid generating all that exception related code inside your function. Right away generated instructions become simpler and cleaner and reduce effect on the instructions that are generated by your actual code where you perform checks:

There is still room for improvement. Since you know that your my_runtime_error won't return you should let the compiler know about it, so that it wouldn't need to preserve registers before calling my_runtime_error:

#if defined(_MSC_VER)
#define NORETURN __declspec(noreturn)
#else
#define NORETURN __attribute__((__noreturn__))
#endif

void NORETURN my_runtime_error(const char* message);
...

When you use it multiple times in your code you can see that generated code is much smaller and reduces effect on instructions that are generated by your actual code:

As you can see, this way compiler doesn't need to preserve registers before calling your my_runtime_error.

I would also suggest against concatenating error strings with __FILE__ and __LINE__ into monolithic error message strings. Pass them as standalone parameters and simply make a macro that passes them along!

void NORETURN my_runtime_error(const char* message, const char* file, int line);
#define MY_ERROR(msg) my_runtime_error(msg, __FILE__, __LINE__)

void test(const char* a)
{
    if (a == nullptr)
        MY_ERROR("error");
    if (a[0] == 'a')
        MY_ERROR("first letter is 'a'");
    if (a[0] == 'b')
        MY_ERROR("first letter is 'b'");
}

It may seem like there is more code generated per each my_runtime_error call (2 more instructions in case of x64 build), but the total size is actually smaller, as the saved size on constant strings is way larger than the extra code size.

Also, note that these code examples are good for showing benefit of making your "hard crashing" function an extern. Need for noreturn becomes more obvious in real code, for example:

#include <math.h>

#if defined(_MSC_VER)
#define NORETURN __declspec(noreturn)
#else
#define NORETURN __attribute__((noreturn))
#endif

void NORETURN my_runtime_error(const char* message, const char* file, int line);
#define MY_ERROR(msg) my_runtime_error(msg, __FILE__, __LINE__)

double test(double x)
{
    int i = floor(x);
    if (i < 10)
        MY_ERROR("error!");
    return 1.0*sqrt(i);
}

Generated assembly:

Try to remove NORETURN, or change __attribute__((noreturn)) to __attribute__((cold)) and you'll see completely different generated assembly!

As a last point (which is obvious IMO and was omitted). You need to define your my_runtime_error function in some cpp file. Since it's going to be one copy only, you can put whatever code you want in this function.

void NORETURN my_runtime_error(const char* message, const char* file, int line)
{
    // you can log the message over network,
    // save it to a file and finally you can throw it an error:
    std::string msg = message;
    msg += " at ";
    msg += file;
    msg += ":";
    msg += std::to_string(line);
    throw std::runtime_error(msg);
}

One more point: clang actually recognizes that this type of function would benefit from noreturn and warns about it if -Wmissing-noreturn warning was enabled:

warning: function 'my_runtime_error' could be declared with attribute 'noreturn' [-Wmissing-noreturn] { ^

Fielding

Thanks for mentioning noreturn - I remembered it after I wrote my answer and have updated it now. – Russo

@JohnZwinck Yes, it really needs noreturn mostly if the function doesn't return. – Fielding

This is quite a definitive guide, Pavel! Great explanation. I have a lot of questions about this approach though: 1. ) Why should I define my own exception for this case? That just decreases the construction cost of the exception(?) which is not something I would want to optimize away. 2. ) Why would my custom except be more efficient than std::runtime_error? 3. ) If my goal is absolute performance, does this mean that I should extract all my throws into noreturn/cold functions? Why aren't throws (that are not handled inside the function) cold by default? – Elijah

4. ) Should I generate separate to-crash-check-throw (for example, prefixed with c_) for my functions, or should I strive to have a general set of checkers. For example, I could make a template function that takes __PRETTY_FUNCTION__ (?) and __LINE__ as template arguments? 5. ) Should I prefer noreturn over cold? Should I use both? What I'm most confused about is why I need the wrapper around the std::runtime error. Shouldn't the definition of MY_ERROR be something like { throw std::runtime_error(msg); }? – Elijah

@AdamHunyadi you don't need to define your own exception class or anything like that. The two most important points are: 1) hide exception code inside some function (I used my_runtime_error for example), 2) mark that function as noreturn to help compiler produce better code. Internally, inside my_runtime_error you can do whatever you want, and throw any exceptions you like, but this function cannot return – Fielding

Answer 2

Yes you can do that. But even better is to move the throw to a separate function, and mark it with __attribute__((cold, noreturn)). This will remove the need to say unlikely() at each call site, and may improve code generation by moving the exception throwing logic entirely outside the happy path, improving instruction cache efficiency and inlining possibilities.

If you prefer to use unlikely() for semantic notation (to make the code easier to read), that's fine too, but it isn't optimal by itself.

Russo

+1, I'm curious however, how will moving the exception throwing logic outside the happy path improve cache efficiency? Is that in reference to how much code is stored in memory, and the fact that reducing the amount of code stored improves cache efficiency? – Archaeopteryx

Interesting. Does cold prevent inlining? – Decisive

@Curious: The exception throwing logic takes space in the generated code. Even in the best case, if that code is shifted to the "end" of a function so it is never encountered, it will still contribute to the total size of the function, which can inhibit inlining. Therefore, small functions that can throw might not be inlined, which means more instructions get executed. Moving it to cold keeps it totally out of the way unless it is needed. – Russo

But don't you think the compiler will change the layout of code like this anyway to be more cache efficient? – Archaeopteryx

@Curious: I too wondered if a sufficiently smart compiler could do this automatically, so I tested it (a long time ago). GCC does not, and it's not entirely clear that it could. Here's one example: godbolt.org/g/HEdwFF - not a perfect one, but even in this simple case you can see how g1() has more instructions in the happy path than g2(). – Russo

@JohnZwinck that is surprising, do you think these flags have something to do with that? -march=haswell --param inline-unit-growth=5 I am not sure what they do :( – Archaeopteryx

@Curious: Those flags aren't very important - you can remove them. – Russo

@JohnZwinck I had hoped gcc does optimize code like that :( – Archaeopteryx

Answer 3

It depends.

First of all, you can definitely do this, and this will likely (pun intended) not harm the performance of your application. But please note that likely/unlikely attributes are compiler-specific, and should be decorated accordingly.

Secondly, if you want a performance gain, the outcome will depend on the target platform (and corresponding compiler backend). If we're talking about the 'default' x86 architecture, you will not get much of a profit on modern chips - the only change these attributes will produce is a change in the code layout (unlike earlier times when x86 supported software branch prediction). For small branches (like your example), it will have very little effect on cache utilization and/or front-end latencies.

UPDATE:

Will the compiler automatically deduce which part of the code should be cached or is this an actually useful thing to do?

This is actually a very wide and complicated topic. What will compiler do depends on the particular compiler, its backend (target architecture) and compilation options. Again, for x86, here's the following rule (taken from Intel® 64 and IA-32 Architectures Optimization Reference Manual):

Assembly/Compiler Coding Rule 3. (M impact, H generality) Arrange code to be consistent with the static branch prediction algorithm: make the fall-through code following a conditional branch be the likely target for a branch with a forward target, and make the fall-through code following a conditional branch be the unlikely target for a branch with a backward target.

As far as I'm aware, that's the only thing that's left from static branch prediction in modern x86, and likely/unlikely attributes might only be used to "overwrite" this default behaviour.

Conchoidal

Why would it need to override? It just needs to pick which branch is first. – Dumps

If likely/unlikely attribute overrides default branch layout, it is applied (at least in GCC). The reason for such behaviour is more laconic C/C++ code layout - simple branch inversion might cause minor performance drawback otherwise. – Conchoidal

Answer 4

Since you're "crashing hard" anyways, I'd go with

#include <cassert>

...
assert(a != nullptr);

This is compiler-independent, should give you close to optimal performance, gives you a breakpoint when running in a debugger, generates a core dump when not in a debugger, and can be disabled by setting the NDEBUG preprocessor symbol, which many build systems do by default for release builds.

Floccose

Assert is by no means an equivalent to exceptions. Also, how are asserts compiler dependent? – Conchoidal

Asserts are better than exceptions for this use case, because they do not destroy the stack -- and they are compiler independent, unlike __builtin_unlikely(). – Floccose

So does this mean that your advice is to not define DNDEBUG in release builds? – Conchoidal

Release builds should be built with -DNDEBUG, because these "fail fast and hard" tests will always lead to data loss for the user. If you want your program to have at least somewhat defined behaviour for internal inconsistencies, you need explicit code for that. – Floccose