| Message ID | 1529603927.Q0ovV4Gg7X@polaris |
|---|---|
| State | New |
| Headers | show |
| Series | Do not emit unnecessary NOPs at -O0 | expand |
On 06/21/2018 11:04 AM, Eric Botcazou wrote: > When code is compiled at -O0, the RTL middle-end makes sure that location > information is preserved as much as possible by generating NOPs with the > location information (goto_locus) present on edges in the CFG, if it thinks > that these edges are the only place where a particular location is mentioned. > > The attached patch prevents this from happening in a couple of cases: > 1. if the function has the DECL_IGNORED_P flag set, > 2. if the NOP is emitted by merge_blocks and the 2nd block is a forwarder > block whose outgoing edge has no location, because in this case the location > of the to-be-elided edge is copied onto the aforementioned outgoing edge. > > Tested (GCC and GDB) on x86-64/Linux, applied on the mainline. > > > 2018-06-21 Eric Botcazou <ebotcazou@adacore.com> > > * cfgrtl.c (fixup_reorder_chain): Do not emit NOPs in DECL_IGNORED_P > functions. > (rtl_merge_blocks): Likewise. Do not emit a NOP if the location of the > edge can be forwarded. > (cfg_layout_merge_blocks): Likewise. Funny Alex and I were just talking about the need to stuff away debug info on edges for gimple. Attaching it to gimple nops sounds like a fairly straightforward approach :-) jeff
On 06/21/2018 11:04 AM, Eric Botcazou wrote: > When code is compiled at -O0, the RTL middle-end makes sure that location > information is preserved as much as possible by generating NOPs with the > location information (goto_locus) present on edges in the CFG, if it thinks > that these edges are the only place where a particular location is mentioned. > > The attached patch prevents this from happening in a couple of cases: > 1. if the function has the DECL_IGNORED_P flag set, > 2. if the NOP is emitted by merge_blocks and the 2nd block is a forwarder > block whose outgoing edge has no location, because in this case the location > of the to-be-elided edge is copied onto the aforementioned outgoing edge. > > Tested (GCC and GDB) on x86-64/Linux, applied on the mainline. > > > 2018-06-21 Eric Botcazou <ebotcazou@adacore.com> > > * cfgrtl.c (fixup_reorder_chain): Do not emit NOPs in DECL_IGNORED_P > functions. > (rtl_merge_blocks): Likewise. Do not emit a NOP if the location of the > edge can be forwarded. > (cfg_layout_merge_blocks): Likewise. > OK Jeff
On Thu, Jun 21, 2018 at 9:37 PM Jeff Law <law@redhat.com> wrote: > > On 06/21/2018 11:04 AM, Eric Botcazou wrote: > > When code is compiled at -O0, the RTL middle-end makes sure that location > > information is preserved as much as possible by generating NOPs with the > > location information (goto_locus) present on edges in the CFG, if it thinks > > that these edges are the only place where a particular location is mentioned. > > > > The attached patch prevents this from happening in a couple of cases: > > 1. if the function has the DECL_IGNORED_P flag set, > > 2. if the NOP is emitted by merge_blocks and the 2nd block is a forwarder > > block whose outgoing edge has no location, because in this case the location > > of the to-be-elided edge is copied onto the aforementioned outgoing edge. > > > > Tested (GCC and GDB) on x86-64/Linux, applied on the mainline. > > > > > > 2018-06-21 Eric Botcazou <ebotcazou@adacore.com> > > > > * cfgrtl.c (fixup_reorder_chain): Do not emit NOPs in DECL_IGNORED_P > > functions. > > (rtl_merge_blocks): Likewise. Do not emit a NOP if the location of the > > edge can be forwarded. > > (cfg_layout_merge_blocks): Likewise. > Funny Alex and I were just talking about the need to stuff away debug > info on edges for gimple. Attaching it to gimple nops sounds like a > fairly straightforward approach :-) But wouldn't that create BBs? Sounds like you want DEBUG PHIs... :/ > > jeff
On 06/22/2018 02:39 AM, Richard Biener wrote: > On Thu, Jun 21, 2018 at 9:37 PM Jeff Law <law@redhat.com> wrote: >> >> On 06/21/2018 11:04 AM, Eric Botcazou wrote: >>> When code is compiled at -O0, the RTL middle-end makes sure that location >>> information is preserved as much as possible by generating NOPs with the >>> location information (goto_locus) present on edges in the CFG, if it thinks >>> that these edges are the only place where a particular location is mentioned. >>> >>> The attached patch prevents this from happening in a couple of cases: >>> 1. if the function has the DECL_IGNORED_P flag set, >>> 2. if the NOP is emitted by merge_blocks and the 2nd block is a forwarder >>> block whose outgoing edge has no location, because in this case the location >>> of the to-be-elided edge is copied onto the aforementioned outgoing edge. >>> >>> Tested (GCC and GDB) on x86-64/Linux, applied on the mainline. >>> >>> >>> 2018-06-21 Eric Botcazou <ebotcazou@adacore.com> >>> >>> * cfgrtl.c (fixup_reorder_chain): Do not emit NOPs in DECL_IGNORED_P >>> functions. >>> (rtl_merge_blocks): Likewise. Do not emit a NOP if the location of the >>> edge can be forwarded. >>> (cfg_layout_merge_blocks): Likewise. >> Funny Alex and I were just talking about the need to stuff away debug >> info on edges for gimple. Attaching it to gimple nops sounds like a >> fairly straightforward approach :-) > > But wouldn't that create BBs? Sounds like you want DEBUG PHIs... :/ We talked about those too :-) Jeff
Index: cfgrtl.c =================================================================== --- cfgrtl.c (revision 261832) +++ cfgrtl.c (working copy) @@ -813,10 +813,14 @@ emit_nop_for_unique_locus_between (basic static void rtl_merge_blocks (basic_block a, basic_block b) { + /* If B is a forwarder block whose outgoing edge has no location, we'll + propagate the locus of the edge between A and B onto it. */ + const bool forward_edge_locus + = (b->flags & BB_FORWARDER_BLOCK) != 0 + && LOCATION_LOCUS (EDGE_SUCC (b, 0)->goto_locus) == UNKNOWN_LOCATION; rtx_insn *b_head = BB_HEAD (b), *b_end = BB_END (b), *a_end = BB_END (a); rtx_insn *del_first = NULL, *del_last = NULL; rtx_insn *b_debug_start = b_end, *b_debug_end = b_end; - bool forwarder_p = (b->flags & BB_FORWARDER_BLOCK) != 0; int b_empty = 0; if (dump_file) @@ -887,9 +891,11 @@ rtl_merge_blocks (basic_block a, basic_b BB_HEAD (b) = b_empty ? NULL : b_head; delete_insn_chain (del_first, del_last, true); - /* When not optimizing and the edge is the only place in RTL which holds - some unique locus, emit a nop with that locus in between. */ - if (!optimize) + /* If not optimizing, preserve the locus of the single edge between + blocks A and B if necessary by emitting a nop. */ + if (!optimize + && !forward_edge_locus + && !DECL_IGNORED_P (current_function_decl)) { emit_nop_for_unique_locus_between (a, b); a_end = BB_END (a); @@ -918,9 +924,7 @@ rtl_merge_blocks (basic_block a, basic_b df_bb_delete (b->index); - /* If B was a forwarder block, propagate the locus on the edge. */ - if (forwarder_p - && LOCATION_LOCUS (EDGE_SUCC (b, 0)->goto_locus) == UNKNOWN_LOCATION) + if (forward_edge_locus) EDGE_SUCC (b, 0)->goto_locus = EDGE_SUCC (a, 0)->goto_locus; if (dump_file) @@ -3916,9 +3920,9 @@ fixup_reorder_chain (void) force_nonfallthru (e); } - /* Ensure goto_locus from edges has some instructions with that locus - in RTL. */ - if (!optimize) + /* Ensure goto_locus from edges has some instructions with that locus in RTL + when not optimizing. */ + if (!optimize && !DECL_IGNORED_P (current_function_decl)) FOR_EACH_BB_FN (bb, cfun) { edge e; @@ -4605,7 +4609,11 @@ cfg_layout_can_merge_blocks_p (basic_blo static void cfg_layout_merge_blocks (basic_block a, basic_block b) { - bool forwarder_p = (b->flags & BB_FORWARDER_BLOCK) != 0; + /* If B is a forwarder block whose outgoing edge has no location, we'll + propagate the locus of the edge between A and B onto it. */ + const bool forward_edge_locus + = (b->flags & BB_FORWARDER_BLOCK) != 0 + && LOCATION_LOCUS (EDGE_SUCC (b, 0)->goto_locus) == UNKNOWN_LOCATION; rtx_insn *insn; gcc_checking_assert (cfg_layout_can_merge_blocks_p (a, b)); @@ -4626,9 +4634,11 @@ cfg_layout_merge_blocks (basic_block a, try_redirect_by_replacing_jump (EDGE_SUCC (a, 0), b, true); gcc_assert (!JUMP_P (BB_END (a))); - /* When not optimizing and the edge is the only place in RTL which holds - some unique locus, emit a nop with that locus in between. */ - if (!optimize) + /* If not optimizing, preserve the locus of the single edge between + blocks A and B if necessary by emitting a nop. */ + if (!optimize + && !forward_edge_locus + && !DECL_IGNORED_P (current_function_decl)) emit_nop_for_unique_locus_between (a, b); /* Move things from b->footer after a->footer. */ @@ -4695,9 +4705,7 @@ cfg_layout_merge_blocks (basic_block a, df_bb_delete (b->index); - /* If B was a forwarder block, propagate the locus on the edge. */ - if (forwarder_p - && LOCATION_LOCUS (EDGE_SUCC (b, 0)->goto_locus) == UNKNOWN_LOCATION) + if (forward_edge_locus) EDGE_SUCC (b, 0)->goto_locus = EDGE_SUCC (a, 0)->goto_locus; if (dump_file)