Hi! AFAIK we use EDGE_ABNORMAL edges (without EDGE_EH) for the following purposes: 1) edges from various spots in the IL to an .ABNORMAL_DISPATCHER basic block 2) edges from that .ABNORMAL_DISPATCHER to bbs with DECL_NONLOCAL label for non-local goto 3) edges from that .ABNORMAL_DISPATCHER to bbs with FORCED_LABEL label for computed jumps 4) edges from that .ABNORMAL_DISPATCHER to bbs starting with __builtin_setjmp_receiver (these actually also have a FORCED_LABEL, and the FORCED_LABEL's address is taken in the __builtin_setjmp_receiver call argument (and corresponding __builtin_setjmp_setup's second argument) 5) edges from that .ABNORMAL_DISPATCHER to bbs starting with a returns_twice call (like normal setjmp, vfork, ...)
4) and 5) above together represent returns_twice calls, which in reality can't be reentered "second" time without actually be called "first" time normally. As the testcase show, we can get ICEs if we remove the normal basic blocks through which they are entered (which could define pre-SSA form SSA_NAMEs that are used after them). If going successfully into SSA form, we should be safe afterwards from this kind of issues, we just have PHIs with (ab) SSA_NAMEs in it, but still if we can prove the returns_twice call is not entered normally, we should be able to remove also the second enter path and anything dominated by that as unreachable; right now that can be eliminated only during RTL opts where we don't have those abnormal edges. The following patch implements this in cleanup_control_flow_pre which is where GIMPLE removal of unreachable basic block happens. For 5), we simply ignore such edges for visited propagation, we have | +---AB2----.ABNORMAL_DISPATCHER | / ^ v v | setjmp AB1 | \ | v +----------------+ normally if it is reachable, so by ignoring the AB2 edge, if there is any normal edge to setjmp, it will be visited (and so will be .ABNORMAL_DISPATCHER and anything that is reachable after setjmp). If the normal incoming edge into setjmp is gone, setjmp will not be visited, even when .ABNORMAL_DISPATCHER is reachable through some other AB edges (we create those pessimistically from lots of spots). For 4), we have | v __builtin_setjmp_setup (..., &<L20>) | \ v +---------AB3------------>.ABNORMAL_DISPATCHER / +---------AB4---------------+ / v <L20>: __builtin_setjmp_receiver (&<L20>) | v The patch ignores the AB4 edge above in normal processing (.ABNORMAL_DISPATCHER could have again many incoming edges from lots of spots, it could be visited before that __builtin_setjmp_setup or after it) but has a special code when we make __builtin_setjmp_setup basic block visited, we queue AB4 edge for special processing where it will not be ignored. And finally, for 1) the patch has a case for a dead .ABNORMAL_DISPATCHER, one that has only incoming edges but no successors. I didn't want to complicate or slow down the processing too much, so it is actually done only if .ABNORMAL_DISPATCHER has no outgoing edges, rather than if it has some, but all of them are in the end to non-visited basic blocks. This means when we for 5) or 4) above remove all such bbs as dead in one cfg cleanup, we remove .ABNORMAL_DISPATCHER only in the following cfg cleanup. Edges 2) and 3) are handled normally as before. The reason the patch tries to be conservative in the checks rather than having gcc_asserts that things are the way they should be is that it seems we unfortunately expose those __builtin_setjmp_setup and __builtin_setjmp_receiver builtins to users and they can do stuff like: void *buf[5]; void foo (int a) { __label__ lab; __builtin_setjmp_setup (buf, &&lab); lab: __builtin_setjmp_receiver (&&lab); // if (a) // __builtin_longjmp (buf, 1); } and it actually compiles (ICEs with the longjmp in there, without/with the following patch; I just didn't want to introduce further ICEs on these bogus testcases). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2019-02-23 Jakub Jelinek <ja...@redhat.com> PR tree-optimization/89280 * tree-cfgcleanup.c (maybe_dead_abnormal_edge_p, builtin_setjmp_setup_bb): New functions. (cleanup_control_flow_pre): Ignore maybe_dead_abnormal_edge_p edges. When visiting __builtin_setjmp_setup block, queue in special setjmp_vec vector edges from .ABNORMAL_DISPATCHER to corresponding __builtin_setjmp_receiver. * gcc.c-torture/compile/pr89280.c: New test. * gcc.dg/torture/pr57147-2.c: Don't expect a setjmp after noreturn function. Skip the test for -O0. --- gcc/tree-cfgcleanup.c.jj 2019-02-23 01:14:03.032266203 +0100 +++ gcc/tree-cfgcleanup.c 2019-02-23 01:40:03.589681687 +0100 @@ -723,6 +723,111 @@ cleanup_tree_cfg_bb (basic_block bb) return false; } +/* Return true if E is an EDGE_ABNORMAL edge for returns_twice calls, + i.e. one going from .ABNORMAL_DISPATCHER to basic block which doesn't + start with a forced or nonlocal label. Calls which return twice can return + the second time only if they are called normally the first time, so basic + blocks which can be only entered through these abnormal edges but not + normally are effectively unreachable as well. Additionally ignore + __builtin_setjmp_receiver starting blocks, which have one FORCED_LABEL + and which are always only reachable through EDGE_ABNORMAL edge. They are + handled in cleanup_control_flow_pre. + Similarly, return true for EDGE_ABNORMAL edge from any basic block to + .ABNORMAL_DISPATCHER basic block if the latter block has no successors. + .ABNORMAL_DISPATCHER basic blocks that don't dispatch to anything are dead, + don't really need any EDGE_ABNORMAL edges to them and can be safely + removed. */ + +static bool +maybe_dead_abnormal_edge_p (edge e) +{ + if ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) != EDGE_ABNORMAL) + return false; + + gimple_stmt_iterator gsi = gsi_start_nondebug_after_labels_bb (e->src); + gimple *g = gsi_stmt (gsi); + if (!g || !gimple_call_internal_p (g, IFN_ABNORMAL_DISPATCHER)) + { + if (EDGE_COUNT (e->dest->succs) == 0) + { + gsi = gsi_start_nondebug_after_labels_bb (e->dest); + g = gsi_stmt (gsi); + if (g && gimple_call_internal_p (g, IFN_ABNORMAL_DISPATCHER)) + return true; + } + return false; + } + + tree target = NULL_TREE; + for (gsi = gsi_start_bb (e->dest); !gsi_end_p (gsi); gsi_next (&gsi)) + if (glabel *label_stmt = dyn_cast <glabel *> (gsi_stmt (gsi))) + { + tree this_target = gimple_label_label (label_stmt); + if (DECL_NONLOCAL (this_target)) + return false; + if (FORCED_LABEL (this_target)) + { + if (target) + return false; + target = this_target; + } + } + else + break; + + if (target) + { + /* If there was a single FORCED_LABEL, check for + __builtin_setjmp_receiver with address of that label. */ + if (!gsi_end_p (gsi) && is_gimple_debug (gsi_stmt (gsi))) + gsi_next_nondebug (&gsi); + if (gsi_end_p (gsi)) + return false; + if (!gimple_call_builtin_p (gsi_stmt (gsi), BUILT_IN_SETJMP_RECEIVER)) + return false; + + tree arg = gimple_call_arg (gsi_stmt (gsi), 0); + if (TREE_CODE (arg) != ADDR_EXPR || TREE_OPERAND (arg, 0) != target) + return false; + } + return true; +} + +/* If BB is a basic block ending with __builtin_setjmp_setup, return edge + from .ABNORMAL_DISPATCHER basic block to corresponding + __builtin_setjmp_receiver basic block, otherwise return NULL. */ +static edge +builtin_setjmp_setup_bb (basic_block bb) +{ + if (EDGE_COUNT (bb->succs) != 2 + || ((EDGE_SUCC (bb, 0)->flags + & (EDGE_ABNORMAL | EDGE_EH)) != EDGE_ABNORMAL + && (EDGE_SUCC (bb, 1)->flags + & (EDGE_ABNORMAL | EDGE_EH)) != EDGE_ABNORMAL)) + return NULL; + + gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb); + if (gsi_end_p (gsi) + || !gimple_call_builtin_p (gsi_stmt (gsi), BUILT_IN_SETJMP_SETUP)) + return NULL; + + tree arg = gimple_call_arg (gsi_stmt (gsi), 1); + if (TREE_CODE (arg) != ADDR_EXPR + || TREE_CODE (TREE_OPERAND (arg, 0)) != LABEL_DECL) + return NULL; + + basic_block recv_bb = label_to_block (cfun, TREE_OPERAND (arg, 0)); + if (EDGE_COUNT (recv_bb->preds) != 1 + || (EDGE_PRED (recv_bb, 0)->flags + & (EDGE_ABNORMAL | EDGE_EH)) != EDGE_ABNORMAL + || (EDGE_SUCC (bb, 0)->dest != EDGE_PRED (recv_bb, 0)->src + && EDGE_SUCC (bb, 1)->dest != EDGE_PRED (recv_bb, 0)->src)) + return NULL; + + /* EDGE_PRED (recv_bb, 0)->src should be the .ABNORMAL_DISPATCHER bb. */ + return EDGE_PRED (recv_bb, 0); +} + /* Do cleanup_control_flow_bb in PRE order. */ static bool @@ -736,10 +841,12 @@ cleanup_control_flow_pre () dom_state saved_state = dom_info_state (CDI_DOMINATORS); set_dom_info_availability (CDI_DOMINATORS, DOM_NONE); - auto_vec<edge_iterator, 20> stack (n_basic_blocks_for_fn (cfun) + 1); + auto_vec<edge_iterator, 20> stack (n_basic_blocks_for_fn (cfun) + 2); auto_sbitmap visited (last_basic_block_for_fn (cfun)); bitmap_clear (visited); + vec<edge, va_gc> *setjmp_vec = NULL; + stack.quick_push (ei_start (ENTRY_BLOCK_PTR_FOR_FN (cfun)->succs)); while (! stack.is_empty ()) @@ -749,12 +856,27 @@ cleanup_control_flow_pre () basic_block dest = ei_edge (ei)->dest; if (dest != EXIT_BLOCK_PTR_FOR_FN (cfun) - && ! bitmap_bit_p (visited, dest->index)) + && !bitmap_bit_p (visited, dest->index) + && (ei_container (ei) == setjmp_vec + || !maybe_dead_abnormal_edge_p (ei_edge (ei)))) { bitmap_set_bit (visited, dest->index); /* We only possibly remove edges from DEST here, leaving possibly unreachable code in the IL. */ retval |= cleanup_control_flow_bb (dest); + + /* Check for __builtin_setjmp_setup. Edges from .ABNORMAL_DISPATCH + to __builtin_setjmp_receiver will be normally ignored by + maybe_dead_abnormal_edge_p. If DEST is a visited + __builtin_setjmp_setup, queue edge from .ABNORMAL_DISPATCH + to __builtin_setjmp_receiver, so that it will be visited too. */ + if (edge e = builtin_setjmp_setup_bb (dest)) + { + vec_safe_push (setjmp_vec, e); + if (vec_safe_length (setjmp_vec) == 1) + stack.quick_push (ei_start (setjmp_vec)); + } + if (EDGE_COUNT (dest->succs) > 0) stack.quick_push (ei_start (dest->succs)); } @@ -763,10 +885,16 @@ cleanup_control_flow_pre () if (!ei_one_before_end_p (ei)) ei_next (&stack.last ()); else - stack.pop (); + { + if (ei_container (ei) == setjmp_vec) + vec_safe_truncate (setjmp_vec, 0); + stack.pop (); + } } } + vec_free (setjmp_vec); + set_dom_info_availability (CDI_DOMINATORS, saved_state); /* We are deleting BBs in non-reverse dominator order, make sure --- gcc/testsuite/gcc.c-torture/compile/pr89280.c.jj 2019-02-23 01:28:52.633838814 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr89280.c 2019-02-23 01:28:52.632838830 +0100 @@ -0,0 +1,48 @@ +/* PR tree-optimization/89280 */ + +int a; +void foo (void); +__attribute__ ((returns_twice)) int bar (void); +void baz (int, int); +void *buf[5]; + +static inline void +inl (int x) +{ + while (x) + foo (); +} + +void +test1 (void) +{ + for (;;) + foo (); + baz (bar (), a); +} + +void +test2 (void) +{ + for (;;) + foo (); + baz (__builtin_setjmp (buf), a); + if (a) + __builtin_longjmp (buf, 1); +} + +void +test3 (void) +{ + inl (1); + baz (bar (), a); +} + +void +test4 (void) +{ + inl (2); + baz (__builtin_setjmp (buf), a); + if (a) + __builtin_longjmp (buf, 1); +} --- gcc/testsuite/gcc.dg/torture/pr57147-2.c.jj 2019-02-23 01:14:03.218263169 +0100 +++ gcc/testsuite/gcc.dg/torture/pr57147-2.c 2019-02-23 01:42:41.529079696 +0100 @@ -1,6 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-fdump-tree-optimized" } */ /* { dg-skip-if "" { *-*-* } { "-fno-fat-lto-objects" } { "" } } */ +/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */ /* { dg-require-effective-target indirect_jumps } */ struct __jmp_buf_tag {}; @@ -19,4 +20,4 @@ void TestSyscall(void) _setjmp (g_return_jmp_buf); } -/* { dg-final { scan-tree-dump "setjmp" "optimized" } } */ +/* { dg-final { scan-tree-dump-not "setjmp" "optimized" } } */ Jakub