Page MenuHomeFreeBSD

cdefs.h Add new __unreachable() builtin
ClosedPublic

Authored by pfg on May 13 2015, 3:49 PM.

Details

Summary

This is one of the post-gcc4.2 builtins that has been implemented by
clang:

__builtin_unreachable is used to indicate that a specific point in the
program cannot be reached, even if the compiler might otherwise think
it can. This is useful to improve optimization and eliminates certain
warnings.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pfg retitled this revision from to cdefs.h Add new __unreachable() builtin.
pfg updated this object.
pfg edited the test plan for this revision. (Show Details)

In general I like this.

As David commented in email:

LLVM uses this quite heavily, in a macro that expands to something equivalent to assert(0 && "unreachable reached!”) in debug mode and __builtin_unreachable() in release mode. When you’re debugging, you get errors if you reach unreachable code and in deployment the compiler gets a useful hint for optimisation.

Should we do something similar in cdefs.h?

In D2536#46858, @emaste wrote:

In general I like this.

As David commented in email:

LLVM uses this quite heavily, in a macro that expands to something equivalent to assert(0 && "unreachable reached!”) in debug mode and __builtin_unreachable() in release mode. When you’re debugging, you get errors if you reach unreachable code and in deployment the compiler gets a useful hint for optimisation.

Should we do something similar in cdefs.h?

Hmm ... I looked at llvm_unreachable() at:
contrib/llvm/include/llvm/Support/ErrorHandling.h

I don't think we want all that in cdefs.

This revision was automatically updated to reflect the committed changes.
In D2536#46863, @pfg wrote:

Hmm ... I looked at llvm_unreachable() at:
contrib/llvm/include/llvm/Support/ErrorHandling.h

I don't think we want all that in cdefs.

I wasn't thinking of anything too elaborate -- perhaps just making it an assertion under #ifndef NDEBUG. But either way I'm happy for this to be in, and we can bikeshed about other changes later on.

In D2536#47048, @emaste wrote:
In D2536#46863, @pfg wrote:

Hmm ... I looked at llvm_unreachable() at:
contrib/llvm/include/llvm/Support/ErrorHandling.h

I don't think we want all that in cdefs.

I wasn't thinking of anything too elaborate -- perhaps just making it an assertion under #ifndef NDEBUG. But either way I'm happy for this to be in, and we can bikeshed about other changes later on.

The builtin is useful for optimizing non-debug cases as well: NetBSD uses it a couple of times in some boot code, but otherwise hasn't adopted it widely. Itś probably better to use it as it is now and as use cases start appearing we can have something more specialized.

In D2536#47068, @pfg wrote:

The builtin is useful for optimizing non-debug cases as well: NetBSD uses it a couple of times in some boot code, but otherwise hasn't adopted it widely. Itś probably better to use it as it is now and as use cases start appearing we can have something more specialized.

Right, that's what I mean -- it can be used as a compiler annotation/hint in release mode, and as an explicit assertion in debug mode.

In D2536#47069, @emaste wrote:
In D2536#47068, @pfg wrote:

The builtin is useful for optimizing non-debug cases as well: NetBSD uses it a couple of times in some boot code, but otherwise hasn't adopted it widely. Itś probably better to use it as it is now and as use cases start appearing we can have something more specialized.

Right, that's what I mean -- it can be used as a compiler annotation/hint in release mode, and as an explicit assertion in debug mode.

Ah, yes .. I see now .. very cool. Feel free to do the enhancement: it appears to be simple but I am not sure how to keep it nice and short ;).