[Development] HEADS-UP: Q_UNLIKELY

[Mutz, Marc]

Hi,

Over the years, compilers (at least GCC) have started to interpret 
Q_UNLIKELY not only as a way to encode information for the branch 
predictor, but more and more to move code marked as such into a 
completely different (.cold) ELF section, which is where the exception 
tables and stack unwinding code also goes. This means two things:

1. Such code is placed far away from the non-unlikely code, which means 
that the normal flow of execution can be more compact, which results in 
an increased effective I-cache size (and, presumably, faster execution).

This we all expect from Q_UNLIKELY and Q_LIKELY.

2. As a separate ELF section, barring relocations etc that might prevent 
this, these .cold sections will stay parked on disk and loaded into 
memory only on demand, which means that executing unlikely code may make 
the program actually very slow (like exceptions, which on modern ABIs 
have zero runtime overhead when not thrown but may load debug symbols to 
unwind the stack when thrown).

This is not something we can control (that I know of, other than telling 
Ville to change it), so we don't need to discuss whether that's good or 
bad. It simply _is_.

In turn, this means that we should no longer use Q_UNLIKELY (I don't 
know about Q_LIKELY, but am willing to be enlightened) to mark code that 
gets executed 0.01% of the time, but with very high chance _is_ executed 
in normal program flow once, because that causes all the error handling 
code to be paged in. We should use it only (but then we should strive to 
actually do so) for actual error code. A good indicator is if a 
qWarning/qFatal/qCritical is executed: that's error code.

Example: https://codereview.qt-project.org/c/qt/qtbase/+/141519

Q_UNLIKELY however, doesn't work on the following code pattern:

     case X: qWarning(...)



     if (foo) {

     } else {
        qWarning(...);
     }



     if (foo) {

         return;
     }

     qWarning(...);

because it requires an expression that then is assumed to unlikely be 
true.

Before C++20, we could reformat the code to invert 'foo' and mark is 
Q_UNLIKELY, and the 141519 change did that in some places. C++20 adds 
[[unlikely]], which doesn't take a condition, so it can be used in the 
above situations. I've proposed it here: 
https://codereview.qt-project.org/c/qt/qtbase/+/272244 This also shows 
some patterns that at least work with GCC. We also know that

     if (foo) [[unlikely]] {}

doesn't work with older GCCs which claim [[unlikely]] support, so we 
will need to either add compiler version checks, or yet another macro, 
or restrict the patterns to the known-good ones:

    Q_UNLIKELY_CASE case-label:

or simply in the flow of execution:

    if (foo) {

    } else {
        Q_UNLIKELY_CASE
        qWarning(...);
    }

    if (foo) {
        return;
    }

    Q_UNLIKELY_CASE
    qWarning(...);

The references patch opts for the latter.

Flame away :)

Thanks,
Marc