Do not emit unnecessary NOPs at -O0

Message ID 1529603927.Q0ovV4Gg7X@polaris
State New
Headers show
Series
  • Do not emit unnecessary NOPs at -O0
Related show

Commit Message

Eric Botcazou June 21, 2018, 5:04 p.m.
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.

-- 
Eric Botcazou

Comments

Jeff Law June 21, 2018, 7:37 p.m. | #1
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
Jeff Law June 21, 2018, 10:38 p.m. | #2
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
Richard Biener June 22, 2018, 8:39 a.m. | #3
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
Jeff Law June 22, 2018, 3:58 p.m. | #4
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

Patch

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)