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

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

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.

On Sat, Mar 12, 2011 at 7:47 PM, Sebastian Redl
<sebastian.redl at getdesigned.at> wrote:
>> 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.
Shall clang hook and override system's assert() somehow?

...Takumi(fine)

On Mar 12, 2011, at 3:01 AM, Duncan Sands wrote:
> 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.