This patch implements the current consensus of PR8973:
http://llvm.org/bugs/show_bug.cgi?id=8973.
The macro llvm_unreachable is used in LLVM to indicate that
a particular place in a function is not supposed to be reachable
during execution. Like an assert macro, it takes a string
argument. In +Asserts builds, this string argument, together with
some location information, is passed to a function which prints
the information and calls abort(). In -Asserts builds, this string
argument is dropped (to minimize code size impact), and
instead a bunch of zero arguments are passed to the same
function.
The problem is that that’s still not very good for code size, as it
leaves a somewhat bulky function call in the emitted code. It
also doesn’t let give the compiler any opportunity to optimize
based on our assertion that the code is unreachable. A much
better alternative is to use an intrinsic, provided by Clang and
GCC 4.5, called __builtin_unreachable; it has the semantics
of being undefined behavior if reached, much like LLVM’s own
“unreachable” instruction, which incidentally is what Clang
generates for it.
This patch keeps the old behavior of llvm_unreachable in
+Asserts (!defined(NDEBUG)) builds, but changes the behavior
in -Asserts builds to call __builtin_unreachable() (in GCC 4.5
and Clang) or abort() (in everything else).
This is effectively a change in the practical semantics of
llvm_unreachable: if the call is actually reachable, then you
will get some really crazy behavior in -Asserts builds. If you’ve
been using this macro in places that can logically be reached —
e.g., after you’ve tested for all the instructions you’ve actually
implemented in your backend — then you’ve been violating the
spirit of this macro, as communicated by its name, and you
should change your code to handle unexpected patterns
more responsibly.
John.
unreachable.patch.txt (1.25 KB)
Such an awesome change it was worth saying twice! 
Sounds great, and I completely agree it’s a nice enhancement to what we can effectively express to help the compiler optimize more effectively. Thanks for doing this.
-Jim
Hi John,
This patch implements the current consensus of PR8973:
http://llvm.org/bugs/show_bug.cgi?id=8973.
The macro llvm_unreachable is used in LLVM to indicate that
a particular place in a function is not supposed to be reachable
during execution. Like an assert macro, it takes a string
argument. In +Asserts builds, this string argument, together with
some location information, is passed to a function which prints
the information and calls abort(). In -Asserts builds, this string
argument is dropped (to minimize code size impact), and
instead a bunch of zero arguments are passed to the same
function.
I have to ask: what is the point of llvm_unreachable? Why not just
use assert?
Ciao, Duncan.
assert completely disappears in release builds, often leading to compiler warnings when the compiler thinks a control path doesn't return a value.
Sebastian
Hi Sebastian,
This patch implements the current consensus of PR8973:
http://llvm.org/bugs/show_bug.cgi?id=8973.
The macro llvm_unreachable is used in LLVM to indicate that
a particular place in a function is not supposed to be reachable
during execution. Like an assert macro, it takes a string
argument. In +Asserts builds, this string argument, together with
some location information, is passed to a function which prints
the information and calls abort(). In -Asserts builds, this string
argument is dropped (to minimize code size impact), and
instead a bunch of zero arguments are passed to the same
function.
I have to ask: what is the point of llvm_unreachable? Why not just
use assert?
assert completely disappears in release builds, often leading to compiler warnings when the compiler thinks a control path doesn't return a value.
if the point is to have a better assert, why not introduce llvm_assert
and do a bulk replace of all asserts with it? By the way, GCC does this:
#ifdef ENABLE_RUNTIME_CHECKING
#define gcc_assert(EXPR) ((void)(!(EXPR) ? abort (), 0 : 0))
#else
/* Include EXPR, so that unused variable warnings do not occur. */
#define gcc_assert(EXPR) ((void)(0 && (EXPR)))
#endif
/* Use gcc_unreachable() to mark unreachable locations (like an
unreachable default case of a switch. Do not use gcc_assert(0). */
#define gcc_unreachable() (abort ())
Ciao, Duncan.
Shall clang hook and override system's assert() somehow?
...Takumi(fine)
if the point is to have a better assert, why not introduce llvm_assert and
do a bulk replace of all asserts with it?
The point is not to have a better assert; it's to have a better unreachable
marker. There are lots of reasons to use a dedicated unreachable marker
instead of assert(0):
- the intent is much more obvious in the code;
- assertions disappear entirely in -Asserts builds, which is obviously
desirable, but we don't necessarily want that for unreachable markers
because of the point Sebastian raised; and
- we can't optimize based on generic assert conditions (*) because it's
extremely common to assert on relatively expensive conditions that
the programmer certainly doesn't want to see evaluated in -Asserts builds.
By contrast, optimizing based on unreachability is quite simple.
(*) I'm aware of research into doing this, but it requires fairly sophisticated
compiler support so that you don't start running those expensive conditions
all the time.
By the way, GCC does this:
Note how GCC also has different macros for normal assertion checks vs.
marking unreachable code, presumably for exactly the reasons above.
We're following that same model, except our unreachable macro:
- takes an explanatory string, which we honor in +Asserts builds, and
- can be optimized in -Asserts builds.
John.
Thanks! Consensus seems to be not opposed, so here we go.
John.