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: Fix CFG WRT __builtin_return


> 
> Hmmm ... I miss changes in is_ctrl_altering_stmt () - and I wonder
> if having an edge to exit is really what we want - currently
> we always have a single predecessor for the exit block (as we
> merge regular returns, and if you don't count fake edges to exit).
> 
> Isn't __builtin_return () rather like a noreturn function?
> (well - it even has attribute noreturn!).  So you'd simply
> have to properly treat it as returning in noreturn discovery?
> (At least I'd have expected your patch to then clear noreturn
> from the builtin).
> 
> It's indeed a somewhat fishy area.  noreturn seems wrong, but
> starting to have multiple real preds of exit sounds like
> asking for trouble as well.  (also PTA does not percieve
> __builtin_return as returning anything ...)

Hi,
we discussed this on IRC, so just for record.  The is_ctrl_altering_stmt
was not needed because all noreturn functions are altering.
This was reason why I originally used this attribute, but I agree
with Richard that it is cleander to remove it.
This needs adjusting in some extra places, in partiuclar in
is_ctrl_altering_stmt and gimple_verify_flow_info.

I also added logic into ipa-pure-const, since function using
built_in_return still can be const.  I don't expect this to
have any thrilling effects, but I need now built_in_return_p
exported for the followup patch fixing noreturn handling in LTO
and this was one place I could quickly think of that would
need special casing for optimization.

Bootstrapped/regtested x86_64-linux, OK?
Honza

	* tree-cfg.c (built_in_return_p): New function.
	(make_edges): Produce edge from BUILT_IN_RETURN
	to exit.
	(execute_warn_function_return): BUILT_IN_RETURN is return.
	(split_critical_edges): Return edges are not critical.
	(is_ctrl_altering_stmt): Builtin_in_return is altering.
	(gimple_verify_flow_info): Handle built_in_return.
	(execute_warn_function_return): Handle built_in_return.
	* ipa-pure-const.c (check_call): Ignore builtin_return.
	* tree-flow.h (built_in_return_p): Declare.

	* gcc.dg/builtin-apply4.c: Compile with -Wmissing-return.
Index: testsuite/gcc.dg/builtin-apply4.c
===================================================================
--- testsuite/gcc.dg/builtin-apply4.c	(revision 160037)
+++ testsuite/gcc.dg/builtin-apply4.c	(working copy)
@@ -1,5 +1,5 @@
 /* PR tree-optimization/20076 */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wmissing-noreturn" } */
 /* { dg-options "-O2 -mno-mmx" { target { { i?86-*-* x86_64-*-* } && ilp32 } } } */
 /* { dg-do run } */
 
Index: builtins.def
===================================================================
--- builtins.def	(revision 160037)
+++ builtins.def	(working copy)
@@ -691,7 +691,7 @@ DEF_GCC_BUILTIN        (BUILT_IN_POPCOUN
 DEF_GCC_BUILTIN        (BUILT_IN_POPCOUNTLL, "popcountll", BT_FN_INT_ULONGLONG, ATTR_CONST_NOTHROW_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_PREFETCH, "prefetch", BT_FN_VOID_CONST_PTR_VAR, ATTR_NOVOPS_LIST)
 DEF_LIB_BUILTIN        (BUILT_IN_REALLOC, "realloc", BT_FN_PTR_PTR_SIZE, ATTR_NOTHROW_LIST)
-DEF_GCC_BUILTIN        (BUILT_IN_RETURN, "return", BT_FN_VOID_PTR, ATTR_NORETURN_NOTHROW_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_RETURN, "return", BT_FN_VOID_PTR, ATTR_NOTHROW_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_RETURN_ADDRESS, "return_address", BT_FN_PTR_UINT, ATTR_NULL)
 DEF_GCC_BUILTIN        (BUILT_IN_SAVEREGS, "saveregs", BT_FN_PTR_VAR, ATTR_NULL)
 DEF_GCC_BUILTIN        (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_NULL)
Index: ipa-pure-const.c
===================================================================
--- ipa-pure-const.c	(revision 160051)
+++ ipa-pure-const.c	(working copy)
@@ -369,6 +369,9 @@ check_call (funct_state local, gimple ca
      graph.  */
   if (callee_t)
     {
+      /* built_in_return is really just an return statemnt.  */
+      if (built_in_return_p (call))
+	return;
       /* When bad things happen to bad functions, they cannot be const
 	 or pure.  */
       if (setjmp_call_p (callee_t))
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 160037)
+++ tree-flow.h	(working copy)
@@ -485,6 +485,7 @@ extern void start_recording_case_labels 
 extern void end_recording_case_labels (void);
 extern basic_block move_sese_region_to_fn (struct function *, basic_block,
 				           basic_block, tree);
+extern bool built_in_return_p (gimple);
 void remove_edge_and_dominated_blocks (edge);
 void mark_virtual_ops_in_bb (basic_block);
 bool tree_node_can_be_shared (tree);
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 160037)
+++ tree-cfg.c	(working copy)
@@ -510,6 +510,18 @@ fold_cond_expr_cond (void)
     }
 }
 
+/* Return true when STMT is call to built_in_return.  */
+
+bool
+built_in_return_p (gimple stmt)
+{
+  tree fndecl;
+  return (is_gimple_call (stmt)
+	  && (fndecl = gimple_call_fndecl (stmt)) != NULL
+	  && DECL_BUILT_IN (fndecl)
+	  && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_RETURN);
+}
+
 /* Join all the blocks in the flowgraph.  */
 
 static void
@@ -568,8 +580,12 @@ make_edges (void)
 		 create abnormal edges to them.  */
 	      make_eh_edges (last);
 
+	      /* BUILTIN_RETURN is really a return statement.  */
+	      if (built_in_return_p (last))
+		make_edge (bb, EXIT_BLOCK_PTR, 0), fallthru = false;
 	      /* Some calls are known not to return.  */
-	      fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
+	      else
+	        fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
 	      break;
 
 	    case GIMPLE_ASSIGN:
@@ -2248,6 +2264,10 @@ is_ctrl_altering_stmt (gimple t)
 	/* A call also alters control flow if it does not return.  */
 	if (flags & ECF_NORETURN)
 	  return true;
+
+	/* BUILT_IN_RETURN call is same as return statement.  */
+	if (built_in_return_p (t))
+	  return true;
       }
       break;
 
@@ -4422,6 +4442,10 @@ gimple_verify_flow_info (void)
 	    }
 	  break;
 
+	case GIMPLE_CALL:
+	  if (!built_in_return_p (stmt))
+	    break;
+	  /* ... fallthru ... */
 	case GIMPLE_RETURN:
 	  if (!single_succ_p (bb)
 	      || (single_succ_edge (bb)->flags
@@ -7036,7 +7060,8 @@ split_critical_edges (void)
 	      gsi = gsi_last_bb (e->src);
 	      if (!gsi_end_p (gsi)
 		  && stmt_ends_bb_p (gsi_stmt (gsi))
-		  && gimple_code (gsi_stmt (gsi)) != GIMPLE_RETURN)
+		  && (gimple_code (gsi_stmt (gsi)) != GIMPLE_RETURN
+		      && !built_in_return_p (gsi_stmt (gsi))))
 		split_edge (e);
 	    }
 	}
@@ -7134,7 +7159,8 @@ execute_warn_function_return (void)
       FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR->preds)
 	{
 	  last = last_stmt (e->src);
-	  if (gimple_code (last) == GIMPLE_RETURN
+	  if ((gimple_code (last) == GIMPLE_RETURN
+	       || built_in_return_p (last))
 	      && (location = gimple_location (last)) != UNKNOWN_LOCATION)
 	    break;
 	}


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