Re: [PATCH] Add new built-in: __builtin_unreachable()

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Jun 3, 2009 at 8:41 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> This patch introduces a new built-in: __builtin_unreachable(). ?The
> motivation is to be able to signal to the compiler that control flow
> never leaves an asm statement.
>
> Back in:
>
> http://gcc.gnu.org/ml/gcc-patches/2000-01/msg00190.html
>
> Zack Weinberg suggested the approach of clobbering the pc in the asm's
> clobber clause. ?This was rejected.
>
> The new __builtin_unreachable() is meant to be placed directly after
> an asm that never returns. ?I cannot think of any other valid uses for
> it, and if control does erroneously reach a __builtin_unreachable(),
> program behavior is completely undefined. ?I think this is acceptable
> behavior as the only reason for having the new built-in is to
> communicate control flow information to the compiler in a case where
> it cannot deduce it.
>
> Consider the case of the Linux kernel's BUG macro/function. ?For the
> default config on x86_64, we currently have:
>
> #define BUG() ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?asm volatile("1:\tud2\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ".pushsection __bug_table,\"a\"\n" ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? __BUG_C0 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? "\t.word %c1, 0\n" ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? "\t.org 2b+%c2\n" ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ".popsection" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? : : "i" (__FILE__), "i" (__LINE__), ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? "i" (sizeof(struct bug_entry))); ? ? ? ? ? \
> ? ? ? ?for (;;) ; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> } while (0)
>
> If we replace that 'for (;;) ;' with a '__builtin_unreachable()', code
> size savings is achieved:
>
> $ size vmlinux.pre vmlinux.post
> ? text ? ?data ? ? bss ? ? dec ? ? hex filename
> 7543782 1122345 ?978944 9645071 ?932c0f vmlinux.pre
> 7532413 1126441 ?978944 9637798 ?930fa6 vmlinux.post
>
> A savings of 7273 bytes.
>
> Similar savings are observed on a mips64 Linux kernel.
>
>
> Bootstrapped and regression tested on x86_64-pc-linux-gnu all default
> languages with no regressions observed.
>
> Also tested by booting x86_64 and mips64 Linux kernels with the
> s/for(;;);/__builtin_unreachable();/ change indicated above.
>
> OK to commit?

With this patch, is

  if (i > 1)
    __builtin_unreachable ();
  if (i > 1)
    ,,,

optimized so that the second test is always false?  Thus, is this patch
suitable to implement assert () for the NDEBUG case?

Thanks,
Richard.


> gcc/
> 2009-06-03 ?David Daney ?<ddaney@caviumnetworks.com>
>
> ? ? ? ?* doc/extend.texi ( __builtin_unreachable): Document new builtin.
> ? ? ? ?* builtins.c (expand_builtin_unreachable): New function.
> ? ? ? ?(expand_builtin): Handle BUILT_IN_UNREACHABLE case.
> ? ? ? ?* builtins.def (BUILT_IN_UNREACHABLE): Add new builtin.
>
> gcc/testsuite/
> 2009-06-03 ?David Daney ?<ddaney@caviumnetworks.com>
>
> ? ? ? ?* gcc.dg/builtin-unreachable.c: New test.
>
> Index: gcc/doc/extend.texi
> ===================================================================
> --- gcc/doc/extend.texi (revision 147888)
> +++ gcc/doc/extend.texi (working copy)
> @@ -6813,6 +6813,38 @@ intentionally executing an illegal instr
> ?you should not rely on any particular implementation.
> ?@end deftypefn
>
> +@deftypefn {Built-in Function} void __builtin_unreachable (void)
> +This function is used to indicate to the compiler that control flow
> +will never reach the point of the @code{__builtin_unreachable}. ?If
> +control flow does reach @code{__builtin_unreachable}, program behavior
> +is undefined. ?The only valid use of this builtin is immediately
> +following an @code{asm} statement that either never exits or transfers
> +control elsewhere never returning. ?In this example, without the
> +@code{__builtin_unreachable}, GCC would issue a warning that control
> +reaches the end of a non-void function. ?It would also generate code
> +to return after the @code{asm}.
> +
> +@smallexample
> +int f (int c, int v)
> +@{
> + ?if (c)
> + ? ?@{
> + ? ? ?return v;
> + ? ?@}
> + ?else
> + ? ?@{
> + ? ? ?asm("jmp error_handler");
> + ? ? ?__builtin_unreachable ();
> + ? ?@}
> +@}
> +@end smallexample
> +
> +Because the @code{asm} statement unconditionally transfers control out
> +of the function, control will never reach the end of the function
> +body. ?The @code{__builtin_unreachable} is in fact unreachable and
> +communicates this fact to the compiler.
> +@end deftypefn
> +
> ?@deftypefn {Built-in Function} void __builtin___clear_cache (char
> *@var{begin}, char *@var{end})
> ?This function is used to flush the processor's instruction cache for
> ?the region of memory between @var{begin} inclusive and @var{end}
> Index: gcc/builtins.c
> ===================================================================
> --- gcc/builtins.c ? ? ?(revision 147888)
> +++ gcc/builtins.c ? ? ?(working copy)
> @@ -5298,6 +5298,16 @@ expand_builtin_trap (void)
> ? emit_barrier ();
> ?}
>
> +/* Expand a call to __builtin_unreachable. ?We do nothing except emit
> + ? the barrier saying that control flow will not pass here. ?It is the
> + ? responsibility of the program being compiled to ensure that control
> + ? flow does never reach here. ?*/
> +static void
> +expand_builtin_unreachable (void)
> +{
> + ?emit_barrier ();
> +}
> +
> ?/* Expand EXP, a call to fabs, fabsf or fabsl.
> ? ?Return NULL_RTX if a normal call should be emitted rather than expanding
> ? ?the function inline. ?If convenient, the result should be placed
> @@ -6795,6 +6805,10 @@ expand_builtin (tree exp, rtx target, rt
> ? ? ? expand_builtin_trap ();
> ? ? ? return const0_rtx;
>
> + ? ?case BUILT_IN_UNREACHABLE:
> + ? ? ?expand_builtin_unreachable ();
> + ? ? ?return const0_rtx;
> +
> ? ? case BUILT_IN_PRINTF:
> ? ? ? target = expand_builtin_printf (exp, target, mode, false);
> ? ? ? if (target)
> Index: gcc/testsuite/gcc.dg/builtin-unreachable.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/builtin-unreachable.c ?(revision 0)
> +++ gcc/testsuite/gcc.dg/builtin-unreachable.c ?(revision 0)
> @@ -0,0 +1,17 @@
> +/* Check that __builtin_unreachable() prevents the 'control reaches
> + ? end of non-void function' diagnostic. ?*/
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wreturn-type" } */
> +int
> +f(int a, int b)
> +{
> + ?if (a)
> + ? ?{
> + ? ? ?return b;
> + ? ?}
> + ?else
> + ? ?{
> + ? ? ?asm ("bug");
> + ? ? ?__builtin_unreachable();
> + ? ?}
> +}
> Index: gcc/builtins.def
> ===================================================================
> --- gcc/builtins.def ? ?(revision 147888)
> +++ gcc/builtins.def ? ?(working copy)
> @@ -698,6 +698,7 @@ DEF_GCC_BUILTIN ? ? ? ?(BUILT_IN_SETJMP,
> ?DEF_EXT_LIB_BUILTIN ? ?(BUILT_IN_STRFMON, "strfmon",
> BT_FN_SSIZE_STRING_SIZE_CONST_STRING_VAR, ATTR_FORMAT_STRFMON_NOTHROW_3_4)
> ?DEF_LIB_BUILTIN ? ? ? ?(BUILT_IN_STRFTIME, "strftime",
> BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR,
> ATTR_FORMAT_STRFTIME_NOTHROW_3_0)
> ?DEF_GCC_BUILTIN ? ? ? ?(BUILT_IN_TRAP, "trap", BT_FN_VOID,
> ATTR_NORETURN_NOTHROW_LIST)
> +DEF_GCC_BUILTIN ? ? ? ?(BUILT_IN_UNREACHABLE, "unreachable", BT_FN_VOID,
> ATTR_NORETURN_NOTHROW_LIST)
> ?DEF_GCC_BUILTIN ? ? ? ?(BUILT_IN_UNWIND_INIT, "unwind_init", BT_FN_VOID,
> ATTR_NULL)
> ?DEF_GCC_BUILTIN ? ? ? ?(BUILT_IN_UPDATE_SETJMP_BUF, "update_setjmp_buf",
> BT_FN_VOID_PTR_INT, ATTR_NULL)
> ?DEF_GCC_BUILTIN ? ? ? ?(BUILT_IN_VA_COPY, "va_copy",
> BT_FN_VOID_VALIST_REF_VALIST_ARG, ATTR_NOTHROW_LIST)
>
>