This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Richard Henderson wrote:
David Daney wrote:
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.



-      if (BARRIER_P (insn))
+      if (BARRIER_P (insn) && prev && !NOTE_INSN_UNREACHABLE_P (prev))

I can tell you that I don't like note-followed-by-barrier meaning something different than barrier by itself. If we wanted a special kind of barrier, I would prefer a flag on the barrier node itself.

That said, I can't figure out why your change to cleanup_barriers
affects anything that would cause crash.  Was this merely to prevent
the barrier from being separated from its note?

Yes. At this point it doesn't really matter, but while debugging the patch, the note was escaping in a different situation and causing CFG verification to fail. I had wanted to prevent that from happening again. But my new patch doesn't use the note so it is moot.



@@ -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);

Egads. Definitely unacceptable.


Let's try

#define BARRIER_BUILTIN_UNREACHABLE(X) \
 (RTL_FLAG_CHECK1("BARRIER_BUILTIN_UNREACHABLE", (X), BARRIER)->volatil)


I think it is not needed.


My initial patch just emitted the barrier, but was causing CFG verification to fail in the case of a block that was empty because it contained only a __builtin_unreachable(). My first attempt to fix the empty block problem was to add the NOTE_INSN_UNREACHABLE that you found objectionable. Since no amount of extra flags on the barrier would solve the empty block issue, I think the best approach is to fix the CFG verifier. That leads to this new patch.

This is exactly the first version of the patch, but we fix rtl_verify_flow_info() so that it properly handles empty blocks when searching for missing barriers.

Tested by bootstrapping on x86_64-pc-linux-gnu all default languages 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-09  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.
	* cfgrtl.c (rtl_verify_flow_info): Handle empty blocks when
	searching for missing barriers.

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

	* gcc.dg/builtin-unreachable-1.c: New test.
	* gcc.dg/builtin-unreachable-2.c: Same.
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 147888)
+++ gcc/doc/extend.texi	(working copy)
@@ -6813,6 +6813,61 @@ 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
+will either never terminate, or one that transfers control elsewhere
+and never returns.  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.
+
+Another use for @code{__builtin_unreachable} is following a call a
+function that never returns but that is not declared
+@code{__attribute__((noreturn))}, as in this example:
+
+@smallexample
+void function_that_never_returns (void);
+
+int g (int c)
+@{
+  if (c)
+    @{
+      return 1;
+    @}
+  else
+    @{
+      function_that_never_returns ();
+      __builtin_unreachable ();
+    @}
+@}
+@end smallexample
+
+@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,17 @@ expand_builtin_trap (void)
   emit_barrier ();
 }
 
+/* Expand a call to __builtin_unreachable.  We do nothing except emit
+   a 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 __builtin_unreachable.  */
+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 +6806,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-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-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/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	(revision 147888)
+++ gcc/cfgrtl.c	(working copy)
@@ -2041,7 +2041,7 @@ rtl_verify_flow_info (void)
 	  for (insn = BB_END (bb); !insn || !BARRIER_P (insn);
 	       insn = NEXT_INSN (insn))
 	    if (!insn
-		|| NOTE_INSN_BASIC_BLOCK_P (insn))
+		|| (NOTE_INSN_BASIC_BLOCK_P (insn) && insn != BB_END (bb)))
 		{
 		  error ("missing barrier after block %i", bb->index);
 		  err = 1;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]