On Fri, May 17, 2013 at 9:52 PM, Jeff Law <l...@redhat.com> wrote: > On 05/17/2013 01:49 PM, Steven Bosscher wrote: >> >> Hello, >> >> Trying to dump the CFG as a graph fails after the pro_and_epilogue >> pass because it leaves unreachable basic blocks. This patch fixes that >> issue. >> >> Will commit sometime next week if no-one objects. >> >> Ciao! >> Steven >> >> >> * function.c (rest_of_handle_thread_prologue_and_epilogue): >> Cleanup the CFG after thread_prologue_and_epilogue_insns. > > ?!? > > Under what conditions does threading the prologue/epilogue make code > unreachable?
Compiling testsuite/gcc.target/i386/pad-10.c with -fdump-rtl-all-graph. Just before rest_of_handle_thread_prologue_and_epilogue we have: Breakpoint 2, rest_of_handle_thread_prologue_and_epilogue () at ../../trunk/gcc/function.c:6969 6969 { (gdb) p brief_dump_cfg(stderr,-1) ;; basic block 2, loop depth 0, count 0, freq 10000, maybe hot ;; prev block 0, next block 3, flags: (REACHABLE, RTL, MODIFIED) ;; pred: ENTRY [100.0%] (FALLTHRU) ;; succ: 3 [9.2%] (FALLTHRU) ;; 4 [90.8%] ;; basic block 3, loop depth 0, count 0, freq 922, maybe hot ;; prev block 2, next block 4, flags: (REACHABLE, RTL, MODIFIED) ;; pred: 2 [9.2%] (FALLTHRU) ;; succ: 4 [100.0%] (FALLTHRU) ;; basic block 4, loop depth 0, count 0, freq 10000, maybe hot ;; prev block 3, next block 1, flags: (REACHABLE, RTL, MODIFIED) ;; pred: 3 [100.0%] (FALLTHRU) ;; 2 [90.8%] ;; succ: EXIT [100.0%] (FALLTHRU) $1 = void (gdb) p debug_bb_n(2) (note 6 1 4 2 [bb 2] NOTE_INSN_BASIC_BLOCK) (note 4 6 31 2 NOTE_INSN_FUNCTION_BEG) (insn 31 4 17 2 (set (reg:SI 0 ax [orig:59 D.1775 ] [59]) (reg/v:SI 4 si [orig:62 x ] [62])) testsuite/gcc.target/i386/pad-10.c:18 85 {*movsi_internal} (nil)) (insn 17 31 8 2 (parallel [ (set (reg:SI 0 ax [orig:59 D.1775 ] [59]) (minus:SI (reg:SI 0 ax [orig:59 D.1775 ] [59]) (reg/v:SI 5 di [orig:61 z ] [61]))) (clobber (reg:CC 17 flags)) ]) testsuite/gcc.target/i386/pad-10.c:18 295 {*subsi_1} (nil)) (insn 8 17 9 2 (set (reg:CCZ 17 flags) (compare:CCZ (reg/v:SI 4 si [orig:62 x ] [62]) (const_int 1 [0x1]))) testsuite/gcc.target/i386/pad-10.c:12 7 {*cmpsi_1} (expr_list:REG_DEAD (reg/v:SI 4 si [orig:62 x ] [62]) (nil))) (jump_insn 9 8 10 2 (set (pc) (if_then_else (ne (reg:CCZ 17 flags) (const_int 0 [0])) (label_ref:DI 18) (pc))) testsuite/gcc.target/i386/pad-10.c:12 602 {*jcc_1} (expr_list:REG_BR_PROB (const_int 9078 [0x2376]) (nil)) -> 18) $2 = (basic_block_def *) 0x7ffff6224410 (gdb) p debug_bb_n(3) (note 10 9 33 3 [bb 3] NOTE_INSN_BASIC_BLOCK) (insn 33 10 11 3 (set (mem/c:SI (plus:DI (reg/f:DI 7 sp) (const_int 12 [0xc])) [2 %sfp+-4 S4 A32]) (reg/v:SI 5 di [orig:61 z ] [61])) 85 {*movsi_internal} (expr_list:REG_DEAD (reg/v:SI 5 di [orig:61 z ] [61]) (nil))) (insn 11 33 12 3 (set (reg:QI 0 ax) (const_int 0 [0])) testsuite/gcc.target/i386/pad-10.c:14 87 {*movqi_internal} (nil)) (call_insn 12 11 34 3 (call (mem:QI (symbol_ref:DI ("bar") [flags 0x41] <function_decl 0x7ffff6225800 bar>) [0 bar S1 A8]) (const_int 0 [0])) testsuite/gcc.target/i386/pad-10.c:14 646 {*call} (expr_list:REG_DEAD (reg:QI 0 ax) (nil)) (expr_list:REG_DEP_TRUE (use (reg:QI 0 ax)) (nil))) (insn 34 12 5 3 (set (reg/v:SI 5 di [orig:61 z ] [61]) (mem/c:SI (plus:DI (reg/f:DI 7 sp) (const_int 12 [0xc])) [2 %sfp+-4 S4 A32])) testsuite/gcc.target/i386/pad-10.c:15 85 {*movsi_internal} (expr_list:REG_DEAD (reg:SI 65) (nil))) (insn 5 34 18 3 (set (reg:SI 0 ax [orig:59 D.1775 ] [59]) (reg/v:SI 5 di [orig:61 z ] [61])) testsuite/gcc.target/i386/pad-10.c:15 85 {*movsi_internal} (expr_list:REG_DEAD (reg/v:SI 5 di [orig:61 z ] [61]) (nil))) $3 = (basic_block_def *) 0x7ffff6224208 (gdb) p debug_bb_n(4) (code_label 18 5 19 4 3 "" [1 uses]) (note 19 18 27 4 [bb 4] NOTE_INSN_BASIC_BLOCK) (insn 27 19 0 4 (use (reg/i:SI 0 ax)) testsuite/gcc.target/i386/pad-10.c:19 -1 (nil)) $4 = (basic_block_def *) 0x7ffff62242d8 Continuing from here leads to an ice in the graph dumper: (gdb) cont Continuing. Breakpoint 1, internal_error (gmsgid=gmsgid@entry=0x119725e "in %s, at %s:%d") at ../../trunk/gcc/diagnostic.c:1091 1091 bool (gdb) bt 10 #0 internal_error (gmsgid=gmsgid@entry=0x119725e "in %s, at %s:%d") at ../../trunk/gcc/diagnostic.c:1091 #1 0x0000000000dfda24 in fancy_abort (file=<optimized out>, line=869, function=0xee3e60 <pre_and_rev_post_order_compute(int*, int*, bool)::__FUNCTION__> "pre_and_rev_post_order_compute") at ../../trunk/gcc/diagnostic.c:1151 #2 0x000000000061dbab in pre_and_rev_post_order_compute (pre_order=0x0, rev_post_order=0x1627850, include_entry_exit=<optimized out>) at ../../trunk/gcc/cfganal.c:869 #3 0x0000000000d28d22 in draw_cfg_nodes_no_loops (fun=<optimized out>, fun=<optimized out>, pp=0x15a4f20 <init_graph_slim_pretty_print(_IO_FILE*)::graph_slim_pp>) at ../../trunk/gcc/graph.c:183 #4 draw_cfg_nodes (fun=0x7ffff6114140, pp=0x15a4f20 <init_graph_slim_pretty_print(_IO_FILE*)::graph_slim_pp>) at ../../trunk/gcc/graph.c:265 #5 print_graph_cfg (base=<optimized out>, fun=0x7ffff6114140) at ../../trunk/gcc/graph.c:306 #6 0x000000000087a330 in execute_one_pass (pass=pass@entry=0x14f6b20 <pass_thread_prologue_and_epilogue>) at ../../trunk/gcc/passes.c:2349 #7 0x000000000087a6a5 in execute_pass_list (pass=0x14f6b20 <pass_thread_prologue_and_epilogue>) at ../../trunk/gcc/passes.c:2378 #8 0x000000000087a6b7 in execute_pass_list (pass=0x14f77a0 <pass_postreload>) at ../../trunk/gcc/passes.c:2379 #9 0x000000000087a6b7 in execute_pass_list (pass=0x14f7800 <pass_rest_of_compilation>) at ../../trunk/gcc/passes.c:2379 (More stack frames follow...) (gdb) p brief_dump_cfg(stderr,-1) ;; basic block 2, loop depth 0, count 0, freq 10000, maybe hot ;; prev block 0, next block 3, flags: (REACHABLE, RTL, MODIFIED) ;; pred: ENTRY [100.0%] (FALLTHRU) ;; succ: 3 [9.2%] (FALLTHRU) ;; 6 [90.8%] ;; basic block 3, loop depth 0, count 0, freq 922, maybe hot ;; prev block 2, next block 4, flags: (REACHABLE, RTL, MODIFIED) ;; pred: 2 [9.2%] (FALLTHRU) ;; succ: 4 [100.0%] (FALLTHRU) ;; basic block 4, loop depth 0, count 0, freq 922, maybe hot ;; prev block 3, next block 6, flags: (REACHABLE, RTL, MODIFIED) ;; pred: 3 [100.0%] (FALLTHRU) ;; succ: 6 [100.0%] (FALLTHRU) ;; basic block 6, loop depth 0, count 0, freq 922, maybe hot ;; Invalid sum of incoming frequencies 10000, should be 922 ;; prev block 4, next block 5, flags: (NEW, RTL, MODIFIED) ;; pred: 4 [100.0%] (FALLTHRU) ;; 2 [90.8%] ;; succ: EXIT [100.0%] ;; basic block 5, loop depth 0, count 0, freq 9078, maybe hot ;; Invalid sum of incoming frequencies 0, should be 9078 ;; prev block 6, next block 1, flags: (NEW, RTL, MODIFIED) ;; pred: ;; succ: EXIT [100.0%] (FAKE) $5 = void (gdb) # Note basic block 5 has no predecessors (gdb) p debug_bb_n(4) (code_label 18 5 19 4 3 "" [0 uses]) (note 19 18 27 4 [bb 4] NOTE_INSN_BASIC_BLOCK) (insn 27 19 45 4 (use (reg/i:SI 0 ax)) testsuite/gcc.target/i386/pad-10.c:19 -1 (nil)) (note 45 27 46 4 NOTE_INSN_EPILOGUE_BEG) (insn/f 46 45 50 4 (parallel [ (set (reg/f:DI 7 sp) (plus:DI (reg/f:DI 7 sp) (const_int 24 [0x18]))) (clobber (reg:CC 17 flags)) (clobber (mem:BLK (scratch) [0 A8])) ]) testsuite/gcc.target/i386/pad-10.c:19 -1 (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:DI 7 sp) (plus:DI (reg/f:DI 7 sp) (const_int 24 [0x18]))) (nil))) $6 = (basic_block_def *) 0x7ffff62242d8 (gdb) p debug_bb_n(5) (code_label 44 48 39 5 5 "" [0 uses]) (note 39 44 43 5 [bb 5] NOTE_INSN_BASIC_BLOCK) (insn 43 39 41 5 (use (reg/i:SI 0 ax)) testsuite/gcc.target/i386/pad-10.c:19 -1 (nil)) $7 = (basic_block_def *) 0x7ffff6224548 (gdb) What's happened, is that emitting the epilogue at the end of basic block 4 (with a barrier at the end) has made the use insn 43 unreachable. The epilogue emitter is not CFG aware, so thread_prologue_and_epilogue_insns has to use find_many_sub_basic_blocks to split blocks it has emitted insns into. find_many_sub_basic_blocks splits basic block 4 and creates basic block 5. The user of find_many_sub_basic_blocks is responsible for cleaning up after h{im,er}self, but thread_prologue_and_epilogue_insns doesn't currently do so. My patch corrects this. I put the cleanup_cfg(0) in rest_of_handle_thread_prologue_and_epilogue because I suspect there are other ways in which thread_prologue_and_epilogue_insns can leave unreachable blocks, especially after shrink-wrapping. > Also note that commit after X days if no-one objects is not the projects > policy. Please don't take such liberties. This reaction seems to me like an unnecessary and disappointing slap on the wrist. Project policy does allow committing obvious patches without review. Perhaps I should just use that policy and not give people the chance to comment? I would think you would also know by now that I know all CFG related code as well as anyone, and, frankly, probably better than even you. Ciao! Steven