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: PR19578: noreturn vs. function pointer propagation part 2


Sorry for taking so long to get back to this...

Diego Novillo <dnovillo@redhat.com> writes:
> Richard Sandiford wrote:
>> 	PR tree-optimization/19578
>> 	* tree-cfg.c (is_ctrl_altering_stmt): Return true for indirect calls.
>>
> Hmm, I guess but I'd like to see some timings first.  What's the compile
> time impact on function-pointer intensive code like libjava/interpret.cc?

OK, I tried interpret.cc, but since it uses exceptions, most of the
indirect calls had an EH edge anyway and were already ending a block.
I couldn't measure any speed difference with the patch.

I ended up trying two artificial testcases instead.  The first was:

    #define A(X) X X X X X X X X X X
    void foo (void (*f) (void))
    {
      A (A (A (A (f ();))));
    }

and the second was:

    #define A(X) X X X X X X X X X X X X X X X X X
    int foo (void (*f) (void))
    {
      int x = 0;
      A (A (A (A (f (); x += 1;))));
      return x;
    }

The patched compiler took about 65% longer to compile the first testcase
at -O2 and about 40% longer to compile the second.  These are only very
rough figures, since there was quite a bit of variation between runs,
but there was definitely a significant and consistent degredation.

> Did you run into trouble with the other approach of marking blocks that
> had function pointers replaced propagated?

In the end, it seemed easiest just to keep a list of modified noreturn
calls, as per the patch below.  This version doesn't seem to affect the
compile time performance for the testcases above, and FWIW, I definitely
prefer it to the original.

My main question with this approach is: once we've identified a mid-block
noreturn, what's the best way of dealing with the following statements?
Since this situation happens so rarely, I decided simply to split the
block after the call (which IMO is exactly what we're doing conceptually).
Deleting the statements is then just another instance of unreachable
block deletion, and we don't need to duplicate any statement management
code.  Let me know if that isn't acceptable though.

Bootstrapped & regression tested on i686-pc-linux-gnu.  OK to install?

Richard


	PR tree-optimization/19578
	* tree-flow.h (modified_noreturn_calls): Declare.
	(noreturn_call_p): Declare.
	* tree-flow-inline.h (noreturn_call_p): New function.
	(modify_stmt): Add modified noreturn calls to modified_noreturn_calls.
	* tree-cfg.c (modified_noreturn_calls): New variable.
	(cleanup_control_flow): Use noreturn_call_p.  Split basic blocks
	that contain a mid-block noreturn call.

Index: tree-flow.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-flow.h,v
retrieving revision 2.78
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.78 tree-flow.h
--- tree-flow.h	24 Jan 2005 20:47:43 -0000	2.78
+++ tree-flow.h	30 Jan 2005 21:37:27 -0000
@@ -294,6 +294,8 @@ union tree_ann_d GTY((desc ("ann_type ((
   struct stmt_ann_d GTY((tag ("STMT_ANN"))) stmt;
 };
 
+extern GTY(()) VEC(tree) *modified_noreturn_calls;
+
 typedef union tree_ann_d *tree_ann_t;
 typedef struct var_ann_d *var_ann_t;
 typedef struct stmt_ann_d *stmt_ann_t;
@@ -307,6 +309,7 @@ static inline stmt_ann_t get_stmt_ann (t
 static inline enum tree_ann_type ann_type (tree_ann_t);
 static inline basic_block bb_for_stmt (tree);
 extern void set_bb_for_stmt (tree, basic_block);
+static inline bool noreturn_call_p (tree);
 static inline void modify_stmt (tree);
 static inline void unmodify_stmt (tree);
 static inline bool stmt_modified_p (tree);
Index: tree-flow-inline.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-flow-inline.h,v
retrieving revision 2.30
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.30 tree-flow-inline.h
--- tree-flow-inline.h	27 Jan 2005 18:22:21 -0000	2.30
+++ tree-flow-inline.h	30 Jan 2005 21:37:27 -0000
@@ -131,6 +131,14 @@ get_filename (tree expr)
     return "???";
 }
 
+/* Return true if T is a noreturn call.  */
+static inline bool
+noreturn_call_p (tree t)
+{
+  tree call = get_call_expr_in (t);
+  return call != 0 && (call_expr_flags (call) & ECF_NORETURN) != 0;
+}
+
 /* Mark statement T as modified.  */
 static inline void
 modify_stmt (tree t)
@@ -138,6 +146,8 @@ modify_stmt (tree t)
   stmt_ann_t ann = stmt_ann (t);
   if (ann == NULL)
     ann = create_stmt_ann (t);
+  else if (noreturn_call_p (t))
+    VEC_safe_push (tree, modified_noreturn_calls, t);
   ann->modified = 1;
 }
 
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.147
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.147 tree-cfg.c
--- tree-cfg.c	22 Jan 2005 17:52:36 -0000	2.147
+++ tree-cfg.c	30 Jan 2005 21:37:28 -0000
@@ -2052,6 +2052,10 @@ remove_bb (basic_block bb)
   remove_phi_nodes_and_edges_for_unreachable_block (bb);
 }
 
+/* A list of all the noreturn calls passed to modify_stmt.  */
+
+VEC(tree) *modified_noreturn_calls;
+
 /* Try to remove superfluous control structures.  */
 
 static bool
@@ -2060,7 +2064,16 @@ cleanup_control_flow (void)
   basic_block bb;
   block_stmt_iterator bsi;
   bool retval = false;
-  tree stmt, call;
+  tree stmt;
+
+  /* Detect cases where a mid-block call is now known not to return.  */
+  while (VEC_length (tree, modified_noreturn_calls))
+    {
+      stmt = VEC_pop (tree, modified_noreturn_calls);
+      bb = bb_for_stmt (stmt);
+      if (bb != NULL && last_stmt (bb) != stmt && noreturn_call_p (stmt))
+	split_block (bb, stmt);
+    }
 
   FOR_EACH_BB (bb)
     {
@@ -2076,10 +2089,7 @@ cleanup_control_flow (void)
 
       /* Check for indirect calls that have been turned into
 	 noreturn calls.  */
-      call = get_call_expr_in (stmt);
-      if (call != 0
-	  && (call_expr_flags (call) & ECF_NORETURN) != 0
-	  && remove_fallthru_edge (bb->succs))
+      if (noreturn_call_p (stmt) && remove_fallthru_edge (bb->succs))
 	{
 	  free_dominance_info (CDI_DOMINATORS);
 	  retval = true;
diff -u /dev/null testsuite/gcc.c-torture/compile/20050130-1.c
--- /dev/null	2005-01-29 16:17:39.000000000 +0000
+++ testsuite/gcc.c-torture/compile/20050130-1.c	2005-01-30 21:01:58.000000000 +0000
@@ -0,0 +1,10 @@
+/* From PR 19578.  */
+extern void foo (void) __attribute__((noreturn));
+
+void
+g (void)
+{
+  void (*f) (void) = foo;
+  f ();
+  f ();
+}


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