llvm dev - Mar 2011 - [LLVMdev] [patch] Change llvm_unreachable to use __builtin_unreachable() in -asserts

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.


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110311/48dff4c4/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: unreachable.patch.txt
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110311/48dff4c4/attachment.txt>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110311/48dff4c4/attachment-0001.html>

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


On Mar 11, 2011, at 6:55 PM, John McCall wrote:
> 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>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110311/d52b46fb/attachment.html>

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.

On 12.03.2011, at 11:17, Duncan Sands wrote:
> 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?
assert completely disappears in release builds, often leading to compiler
warnings when the compiler thinks a control path doesn't return a value.

Sebastian

On Mar 11, 2011, at 7:45 PM, Jim Grosbach wrote:
> 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.
Thanks!  Consensus seems to be not opposed, so here we go. :)

John.