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:
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.
...
@@ -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)))

I'd prefer to have this section rewritten as


@@ -2045,16 +2045,18 @@ rtl_verify_flow_info (void)
{
rtx insn;
- /* Ensure existence of barrier in BB with no fallthru edges. */
- for (insn = BB_END (bb); !insn || !BARRIER_P (insn);
- insn = NEXT_INSN (insn))
- if (!insn
- || NOTE_INSN_BASIC_BLOCK_P (insn))
+ /* Ensure existence of barrier after BB with no fallthru edges. */
+ for (insn = NEXT_INSN (BB_END (bb)); ; insn = NEXT_INSN (insn))
+ {
+ if (!insn || NOTE_INSN_BASIC_BLOCK_P (insn))
{
error ("missing barrier after block %i", bb->index);
err = 1;
break;
}
+ if (BARRIER_P (insn))
+ break;
+ }
}
else if (e->src != ENTRY_BLOCK_PTR
&& e->dest != EXIT_BLOCK_PTR)

Note that this version never checks vs BB_END.



Ok, that is cleaner.


I'd also like you to look into why gcc -O on your
builtin-unreachable-2.c test still produces a branch to next:

        cmpl    $1, %edi
        jle     .L2
.L2:
        movl    $1, %eax
        ret


It was jumping around the empty block with a barrier. I modified the patch to delete such blocks in try_optimize_cfg(). The resulting code omits the useless comparison and jump.



If we are to use this construct with assert.h NDEBUG to give the
compiler extra information with which to optimize, we'd really
like the test itself to be optimized away as well.  At the gimple
level the __builtin_unreachable function call will still be
present, and so the gimple optimizers should be able to take
advantage of the (i > 1) test, but at the rtl level I think we'd
like the totally empty block (and the branch around it) to get
folded away.

Agreed, and now they are.


How about this version. Tested with all default languages on x86_64 with no regressions.

OK to commit?

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

	PR c/39252
	* 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.
	* cfgcleanup.c (try_optimize_cfg): Delete empty blocks with no
	successors.
	* cfgrtl.c (rtl_verify_flow_info): Handle empty blocks when
	searching for missing barriers.

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

	PR c/39252
	* 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,20 @@
+/* Check that __builtin_unreachable() is a no-return function thus
+   causing the dead call to foo() to be removed.  The comparison is
+   dead too, and should be removed.  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized -fdump-rtl-cse1" } */
+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 { scan-rtl-dump-not "\\(if_then_else" "cse1" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+/* { dg-final { cleanup-rtl-dump "cse1" } } */
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/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c	(revision 147888)
+++ gcc/cfgcleanup.c	(working copy)
@@ -1874,8 +1874,12 @@ try_optimize_cfg (int mode)
 	      edge s;
 	      bool changed_here = false;
 
-	      /* Delete trivially dead basic blocks.  */
-	      if (EDGE_COUNT (b->preds) == 0)
+	      /* Delete trivially dead basic blocks.  This is either
+		 blocks with no predecessors, or empty blocks with no
+		 successors.  Empty blocks may result from expanding
+		 __builtin_unreachable ().  */
+	      if (EDGE_COUNT (b->preds) == 0
+		  || (EDGE_COUNT (b->succs) == 0 && BB_HEAD (b) == BB_END (b)))
 		{
 		  c = b->prev_bb;
 		  if (dump_file)
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	(revision 147888)
+++ gcc/cfgrtl.c	(working copy)
@@ -2038,15 +2038,17 @@ rtl_verify_flow_info (void)
 	  rtx insn;
 
 	  /* Ensure existence of barrier in BB with no fallthru edges.  */
-	  for (insn = BB_END (bb); !insn || !BARRIER_P (insn);
-	       insn = NEXT_INSN (insn))
-	    if (!insn
-		|| NOTE_INSN_BASIC_BLOCK_P (insn))
+	  for (insn = NEXT_INSN (BB_END (bb)); ; insn = NEXT_INSN (insn))
+	    {
+	      if (!insn || NOTE_INSN_BASIC_BLOCK_P (insn))
 		{
 		  error ("missing barrier after block %i", bb->index);
 		  err = 1;
 		  break;
 		}
+	      if (BARRIER_P (insn))
+		break;
+	    }
 	}
       else if (e->src != ENTRY_BLOCK_PTR
 	       && e->dest != EXIT_BLOCK_PTR)

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