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]

PR tree-opt/19484: noreturn vs. function pointer propagation


This is my first gimple patch, so please be gentle...

It fixes the following testcase from PR 19484 (which was derived
from glibc CVS):

    extern void foo (void) __attribute__((noreturn));
    int n;

    void
    g (void)
    {
      void (*f) (void) = foo;
      if (n)
        f ();
      n = 1;
    }

The tree optimisers will rewrite "f ()" as "foo ()", but they
don't update the cfg to reflect the fact that the latter never
returns.  This causes a checking failure during rtl expansion.

The proposed fix is to treat this in a similar way to COND_EXPRs or
SWITCH_EXPRs whose conditions become known constants.  Specifically,
make cleanup_control_flow() look for blocks that end in no-return
CALL_EXPRs and remove any fallthru edges from them.  Patch below.

Unfortunately, that isn't enough to fix the second testcase in the PR
(which was just written from inspection, I don't know of any real code
like this):

    extern void foo (void) __attribute__((noreturn));

    void g (void)
    {
      void (*f) (void) = foo;
      f ();
      f ();
    }

The first CALL_EXPR is of course in the middle of a block, so rewriting
it as foo () leads to:

    error: Control flow in the middle of basic block 0

I don't know how best to deal with this.  Some random alternatives
(by no means a complete list):

  (a) Forbid the replacement.  (Is there a predicate that already
      checks for things like this?  I.e. which examines the context
      of the replacement, not just the operands being replaced?)

  (b) Extend cleanup_control_flow() so that it removes dead code after
      non-returning CALL_EXPRs.  Probably not a contender because the
      function would then be O(#statements) rather than O(#blocks).

  (c) Like (b), but only do the extra work when at least one CALL_EXPR
      is known to be affected.  (What would be the best way to record this?
      Some sort of global flag set by modify_stmt?  A new basic block flag?)

  (d) Like (c), but actually record which CALL_EXPRs are affected.
      (Some sort of global list instead of a global flag?)

  (e) Remove the dead code in some other pass.

  etc.

What would be the Gimple Way here?

Anyway, patch boostrapped & regression tested on i686-pc-linux-gnu.
I wanted to make sure the new code handles abnormal edges correctly,
so there are two testcases: the original, and one with a nonlocal goto.
OK to install?

If this patch is OK, I'll close 19484 and open another PR for the
second testcase quoted above.

Richard


	PR tree-optimization/19484
	* tree-cfg.c (remove_fallthru_edge): New function.
	(cleanup_control_flow): Remove fallthru edges from calls that are
	now known not to return.

Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.139
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.139 tree-cfg.c
*** tree-cfg.c	17 Jan 2005 18:44:18 -0000	2.139
--- tree-cfg.c	20 Jan 2005 12:59:17 -0000
*************** static void make_goto_expr_edges (basic_
*** 114,119 ****
--- 114,120 ----
  static edge tree_redirect_edge_and_branch (edge, basic_block);
  static edge tree_try_redirect_by_replacing_jump (edge, basic_block);
  static void split_critical_edges (void);
+ static bool remove_fallthru_edge (VEC(edge) *);
  
  /* Various helpers.  */
  static inline bool stmt_starts_bb_p (tree, tree);
*************** cleanup_control_flow (void)
*** 2059,2065 ****
    basic_block bb;
    block_stmt_iterator bsi;
    bool retval = false;
!   tree stmt;
  
    FOR_EACH_BB (bb)
      {
--- 2060,2066 ----
    basic_block bb;
    block_stmt_iterator bsi;
    bool retval = false;
!   tree stmt, call;
  
    FOR_EACH_BB (bb)
      {
*************** cleanup_control_flow (void)
*** 2072,2077 ****
--- 2073,2089 ----
        if (TREE_CODE (stmt) == COND_EXPR
  	  || TREE_CODE (stmt) == SWITCH_EXPR)
  	retval |= cleanup_control_expr_graph (bb, bsi);
+ 
+       /* 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))
+ 	{
+ 	  free_dominance_info (CDI_DOMINATORS);
+ 	  retval = true;
+ 	}
      }
    return retval;
  }
*************** cleanup_control_expr_graph (basic_block 
*** 2140,2145 ****
--- 2152,2173 ----
    return retval;
  }
  
+ /* Remove any fallthru edge from EV.  Return true if an edge was removed.  */
+ 
+ static bool
+ remove_fallthru_edge (VEC(edge) *ev)
+ {
+   edge_iterator ei;
+   edge e;
+ 
+   FOR_EACH_EDGE (e, ei, ev)
+     if ((e->flags & EDGE_FALLTHRU) != 0)
+       {
+ 	remove_edge (e);
+ 	return true;
+       }
+   return false;
+ }
  
  /* Given a basic block BB ending with COND_EXPR or SWITCH_EXPR, and a
     predicate VAL, return the edge that will be taken out of the block.
diff -c /dev/null testsuite/gcc.c-torture/compile/20050119-1.c
*** /dev/null	2005-01-19 11:38:20.753181944 +0000
--- testsuite/gcc.c-torture/compile/20050119-1.c	2005-01-20 12:43:30.000000000 +0000
***************
*** 0 ****
--- 1,11 ----
+ extern void foo (void) __attribute__((noreturn));
+ int n;
+ 
+ void
+ g (void)
+ {
+   void (*f) (void) = foo;
+   if (n)
+     f ();
+   n = 1;
+ }
diff -c /dev/null testsuite/gcc.c-torture/compile/20050119-2.c
*** /dev/null	2005-01-19 11:38:20.753181944 +0000
--- testsuite/gcc.c-torture/compile/20050119-2.c	2005-01-20 12:43:30.000000000 +0000
***************
*** 0 ****
--- 1,18 ----
+ extern void foo (void) __attribute__((noreturn));
+ int n;
+ 
+ void
+ g (void)
+ {
+   __label__ lab;
+   void h (void) { if (n == 2) goto lab; }
+   void (*f1) (void) = foo;
+   void (*f2) (void) = h;
+ 
+   f2 ();
+   if (n)
+     f1 ();
+   n = 1;
+  lab:
+   n++;
+ }


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