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

[David Daney <ddaney at caviumnetworks dot com>]

Ian Lance Taylor wrote:
> David Daney <ddaney@caviumnetworks.com> writes:
>
> > +@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.
>
> I have wanted __builtin_unreachable for a while, but never got around to
> doing anything.  But I only want it if it works in general, not just
> after an asm statement.  I think it should mean "if control flow reaches
> this point, the program is undefined."  Ideally there should be a
> command line option to control it, so that it is possible to compile
> __builtin_unreachable as expanding to abort, or something like that,
> when debugging.

Ok, then how about this version?  All my original commentary still 
applies: http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00317.html

I have added a new command line flag (-funreachable-traps) that causes 
__builtin_unreachable() to be expanded exactly as if it were a 
__builtin_trap().


Tested by bootstrapping and regression testing all default languages on 
x86_64-pc-linux-gnu with no regressions.  Also tested by building and 
running the Linux kernel for x86_64 and mips64 after replacing the 
for(;;) with __builtin_unreachable() in their respective BUG() 
macros/functions.

OK to commit?

gcc/
2009-06-08  David Daney  <ddaney@caviumnetworks.com>

	* doc/extend.texi ( __builtin_unreachable): Document new builtin.
	* doc/invoke.texi (-funreachable-traps): Document new option.
	* doc/rtl.texi (NOTE_INSN_UNREACHABLE): Document new note type.
	* final.c (final_scan_insn): Handle NOTE_INSN_UNREACHABLE.
	* builtins.c (expand_builtin_unreachable): New function.
	(expand_builtin): Handle BUILT_IN_UNREACHABLE case.
	* insn-notes.def (UNREACHABLE): Add new INSN_NOTE type.
	* builtins.def (BUILT_IN_UNREACHABLE): Add new builtin.
	* jump.c (cleanup_barriers): Don't reorder NOTE_INSN_UNREACHABLE
	across a barrier.
	* cfgbuild.c (control_flow_insn_p): Return true for
	NOTE_INSN_UNREACHABLE.
	* cfgcleanup.c (old_insns_match_p): Return true if both insns are
	NOTE_INSN_UNREACHABLE.
	* common.opt (funreachable-traps): Add new option.
	* rtl.h (NOTE_INSN_UNREACHABLE_P): New macro.
	* cfgrtl.c (can_delete_note_p): Return true for NOTE_INSN_UNREACHABLE.

gcc/testsuite/
2009-06-08  David Daney  <ddaney@caviumnetworks.com>

	* gcc.dg/builtin-unreachable-1.c: New test.
	* gcc.dg/builtin-unreachable-2.c: Same.
	* gcc.dg/builtin-unreachable-3.c: Same.

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 147888)
+++ gcc/doc/extend.texi	(working copy)
@@ -6813,6 +6813,43 @@ intentionally executing an illegal instr
 you should not rely on any particular implementation.
 @end deftypefn
 
+@deftypefn {Built-in Function} void __builtin_unreachable (void)
+If control flow reaches the point of the @code{__builtin_unreachable ();},
+the program is undefined.  It is useful in situations where the
+compiler cannot deduce the unreachability of the code.  One such case
+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.
+
+As a debugging aid, you can use the @option{-funreachable-traps} flag
+when compiling.  This causes @code{__builtin_unreachable ();} to be
+treated as a @code{__builtin_trap ();}, thus causing the program to exit
+abnormally in the case that control flow would reach a
+@code{__builtin_unreachable ();}.
+@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/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 147888)
+++ gcc/doc/invoke.texi	(working copy)
@@ -867,7 +867,7 @@ See S/390 and zSeries Options.
 -fargument-noalias-global  -fargument-noalias-anything @gol
 -fleading-underscore  -ftls-model=@var{model} @gol
 -ftrapv  -fwrapv  -fbounds-check @gol
--fvisibility}
+-fvisibility -funreachable-traps}
 @end table
 
 @menu
@@ -16340,6 +16340,14 @@ the DSOs.
 An overview of these techniques, their benefits and how to use them
 is at @w{@uref{http://gcc.gnu.org/wiki/Visibility}}.
 
+@item -funreachable-traps
+@opindex funreachable-traps
+Treat @code{__builtin_unreachable ();} as if it were
+@code{__builtin_trap ();}.  When this option is used, the program will
+exit abnormally if control flow reaches a
+@code{__builtin_unreachable ();}.  This can be used as a debugging aid
+to help verify that the @code{__builtin_unreachable ();} is never reached.
+
 @end table
 
 @c man end
Index: gcc/doc/rtl.texi
===================================================================
--- gcc/doc/rtl.texi	(revision 147888)
+++ gcc/doc/rtl.texi	(working copy)
@@ -3527,6 +3527,11 @@ invariants.
 Appears at the start of the function body, after the function
 prologue.
 
+@findex NOTE_INSN_UNREACHABLE
+@item NOTE_INSN_UNREACHABLE
+This note indicates the point a @code{__builtin_unreachable ();} was
+expanded.
+
 @end table
 
 These codes are printed symbolically when they appear in debugging dumps.
Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 147888)
+++ gcc/final.c	(working copy)
@@ -1821,6 +1821,7 @@ final_scan_insn (rtx insn, FILE *file, i
       switch (NOTE_KIND (insn))
 	{
 	case NOTE_INSN_DELETED:
+	case NOTE_INSN_UNREACHABLE:
 	  break;
 
 	case NOTE_INSN_SWITCH_TEXT_SECTIONS:
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 147888)
+++ gcc/builtins.c	(working copy)
@@ -5298,6 +5298,29 @@ expand_builtin_trap (void)
   emit_barrier ();
 }
 
+/* Expand a call to __builtin_unreachable.  If -funreachable-traps was
+   specified, expand as __builtin_trap(), otherwise we do nothing
+   except emit a note and the barrier saying that control flow will
+   not pass here.  The note is needed so that the last insn in a basic
+   block containing only __builtin_unreachable() is not the
+   NOTE_INSN_BASIC_BLOCK, which is not allowed in the rtl cfg.
+
+   It is the responsibility of the program being compiled to ensure
+   that control flow does never reach here.  */
+static void
+expand_builtin_unreachable (void)
+{
+  if (flag_unreachable_traps)
+    {
+      expand_builtin_trap();
+    }
+  else
+    {
+      emit_note (NOTE_INSN_UNREACHABLE);
+      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 +6818,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/insn-notes.def
===================================================================
--- gcc/insn-notes.def	(revision 147888)
+++ gcc/insn-notes.def	(working copy)
@@ -70,4 +70,7 @@ INSN_NOTE (BASIC_BLOCK)
    between hot and cold text sections.  */
 INSN_NOTE (SWITCH_TEXT_SECTIONS)
 
+/* Mark the point where a __builtin_unreachable() was expanded.  */
+INSN_NOTE (UNREACHABLE)
+
 #undef INSN_NOTE
Index: gcc/testsuite/gcc.dg/builtin-unreachable-2.c
===================================================================
--- gcc/testsuite/gcc.dg/builtin-unreachable-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/builtin-unreachable-2.c	(revision 0)
@@ -0,0 +1,17 @@
+/* Check that __builtin_unreachable() is a no-return
+   function thus causing the dead call to foo() to be removed.  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+void foo (void);
+
+int
+f (int i)
+{
+  if (i > 1)
+    __builtin_unreachable();
+  if (i > 1)
+    foo ();
+  return 1;
+}
+/* { dg-final { scan-tree-dump-not "foo" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: gcc/testsuite/gcc.dg/builtin-unreachable-3.c
===================================================================
--- gcc/testsuite/gcc.dg/builtin-unreachable-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/builtin-unreachable-3.c	(revision 0)
@@ -0,0 +1,11 @@
+/* Check that -funreachable-traps converts __builtin_unreachable()
+   into __builtin_trap().  */
+/* { dg-do compile { target x86_64-*-* i?86-*-* powerpc*-*-* arm*-*-* mips*-*-* } } */
+/* { dg-options "-O2 -funreachable-traps -fdump-rtl-mach" } */
+int
+f (int i)
+{
+  __builtin_unreachable();
+}
+/* { dg-final { scan-rtl-dump "trap_if" "mach" } } */
+/* { dg-final { cleanup-rtl-dump "mach" } } */
Index: gcc/testsuite/gcc.dg/builtin-unreachable-1.c
===================================================================
--- gcc/testsuite/gcc.dg/builtin-unreachable-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/builtin-unreachable-1.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)
Index: gcc/jump.c
===================================================================
--- gcc/jump.c	(revision 147888)
+++ gcc/jump.c	(working copy)
@@ -107,10 +107,11 @@ unsigned int
 cleanup_barriers (void)
 {
   rtx insn, next, prev;
+  prev = NULL;
   for (insn = get_insns (); insn; insn = next)
     {
       next = NEXT_INSN (insn);
-      if (BARRIER_P (insn))
+      if (BARRIER_P (insn) && prev && !NOTE_INSN_UNREACHABLE_P (prev))
 	{
 	  prev = prev_nonnote_insn (insn);
 	  if (BARRIER_P (prev))
@@ -118,6 +119,7 @@ cleanup_barriers (void)
 	  else if (prev != PREV_INSN (insn))
 	    reorder_insns (insn, insn, prev);
 	}
+      prev = insn;
     }
   return 0;
 }
Index: gcc/cfgbuild.c
===================================================================
--- gcc/cfgbuild.c	(revision 147888)
+++ gcc/cfgbuild.c	(working copy)
@@ -84,6 +84,9 @@ control_flow_insn_p (const_rtx insn)
   switch (GET_CODE (insn))
     {
     case NOTE:
+      /* __builtin_unreachable is treated like a noreturn function. */
+      return NOTE_INSN_UNREACHABLE_P (insn);
+
     case CODE_LABEL:
       return false;
 
Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c	(revision 147888)
+++ gcc/cfgcleanup.c	(working copy)
@@ -953,6 +953,11 @@ old_insns_match_p (int mode ATTRIBUTE_UN
   if (GET_CODE (i1) != GET_CODE (i2))
     return false;
 
+  /* Being unreachable doesn't prevent crossjumping to preceding
+     insns.  */
+  if (NOTE_INSN_UNREACHABLE_P (i1) && NOTE_INSN_UNREACHABLE_P (i2))
+    return true;
+
   p1 = PATTERN (i1);
   p2 = PATTERN (i2);
 
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 147888)
+++ gcc/common.opt	(working copy)
@@ -1274,6 +1274,10 @@ ftree-vrp
 Common Report Var(flag_tree_vrp) Init(0) Optimization
 Perform Value Range Propagation on trees
 
+funreachable-traps
+Common Report Var(flag_unreachable_traps)
+Treat __builtin_unreachable() as if it were __builtin_trap().
+
 funit-at-a-time
 Common Report Var(flag_unit_at_a_time) Init(1) Optimization
 Compile whole compilation unit at a time
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	(revision 147888)
+++ gcc/rtl.h	(working copy)
@@ -871,6 +871,12 @@ extern const char * const reg_note_name[
   (GET_CODE (INSN) == NOTE				\
    && NOTE_KIND (INSN) == NOTE_INSN_BASIC_BLOCK)
 
+/* Nonzero if INSN is a note marking the expansion of
+   __builtin_unreachable().  */
+#define NOTE_INSN_UNREACHABLE_P(INSN)			\
+  (GET_CODE (INSN) == NOTE				\
+   && NOTE_KIND (INSN) == NOTE_INSN_UNREACHABLE)
+
 /* Variable declaration and the location of a variable.  */
 #define NOTE_VAR_LOCATION_DECL(INSN)	(XCTREE (XCEXP (INSN, 4, NOTE), \
 						 0, VAR_LOCATION))
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	(revision 147888)
+++ gcc/cfgrtl.c	(working copy)
@@ -87,7 +87,8 @@ static int
 can_delete_note_p (const_rtx note)
 {
   return (NOTE_KIND (note) == NOTE_INSN_DELETED
-	  || NOTE_KIND (note) == NOTE_INSN_BASIC_BLOCK);
+	  || NOTE_KIND (note) == NOTE_INSN_BASIC_BLOCK
+	  || NOTE_KIND (note) == NOTE_INSN_UNREACHABLE);
 }
 
 /* True if a given label can be deleted.  */