diff mbox

Save and restore EDGE_DFS_BACK in draw_cfg_edges

Message ID 506e05c1-9167-a3f4-1451-8886863bef38@mentor.com
State New
Headers show

Commit Message

Tom de Vries July 3, 2017, 10:41 p.m. UTC 
[was: Re: [PATCH] Add dotfn ]

On 07/03/2017 12:23 PM, Richard Biener wrote:
>> Btw, I think this needs fixing:
>> ...
>> /* Draw all edges in the CFG.  Retreating edges are drawin as not
>>     constraining, this makes the layout of the graph better.
>>     (??? Calling mark_dfs_back may change the compiler's behavior when
>>     dumping, but computing back edges here for ourselves is also not
>>     desirable.)  */
>>
>> static void
>> draw_cfg_edges (pretty_printer *pp, struct function *fun)
>> {
>>    basic_block bb;
>>    mark_dfs_back_edges ();
>>    FOR_ALL_BB_FN (bb, cfun)
>>      draw_cfg_node_succ_edges (pp, fun->funcdef_no, bb);
>> ...
>>
>> We don't want that calling a debug function changes compiler behavior
>> (something I ran into while debugging PR81192).
>>
>> Any suggestion on how to address this? We could allocate a bitmap before and
>> save the edge flag for all edges, and restore afterwards.

> Something like that, yes.
> 

This patch implements that approach.

I've tried it with the PR81192 example and calling DOTFN in tail-merge, 
like this:
1. Just compiling the example without any patches gives a tail-merge
    sigsegv.
2. Compiling with the DOTFN call in tail-merge makes the sigsegv go
    away.
3. Adding this patch makes the sigsegv come back.

OK for trunk if bootstrap and reg-test on x86_64 succeeds?

Thanks,
- Tom

Comments

Richard Biener July 4, 2017, 7:03 a.m. UTC | #1 
On Tue, 4 Jul 2017, Tom de Vries wrote:

> [was: Re: [PATCH] Add dotfn ]
> 
> On 07/03/2017 12:23 PM, Richard Biener wrote:
> > > Btw, I think this needs fixing:
> > > ...
> > > /* Draw all edges in the CFG.  Retreating edges are drawin as not
> > >     constraining, this makes the layout of the graph better.
> > >     (??? Calling mark_dfs_back may change the compiler's behavior when
> > >     dumping, but computing back edges here for ourselves is also not
> > >     desirable.)  */
> > > 
> > > static void
> > > draw_cfg_edges (pretty_printer *pp, struct function *fun)
> > > {
> > >    basic_block bb;
> > >    mark_dfs_back_edges ();
> > >    FOR_ALL_BB_FN (bb, cfun)
> > >      draw_cfg_node_succ_edges (pp, fun->funcdef_no, bb);
> > > ...
> > > 
> > > We don't want that calling a debug function changes compiler behavior
> > > (something I ran into while debugging PR81192).
> > > 
> > > Any suggestion on how to address this? We could allocate a bitmap before
> > > and
> > > save the edge flag for all edges, and restore afterwards.
> 
> > Something like that, yes.
> > 
> 
> This patch implements that approach.
> 
> I've tried it with the PR81192 example and calling DOTFN in tail-merge, like
> this:
> 1. Just compiling the example without any patches gives a tail-merge
>    sigsegv.
> 2. Compiling with the DOTFN call in tail-merge makes the sigsegv go
>    away.
> 3. Adding this patch makes the sigsegv come back.
> 
> OK for trunk if bootstrap and reg-test on x86_64 succeeds?

You don't need to iterate over both preds and succs, succs
is enough.

Ok with that change.

Richard.

> Thanks,
> - Tom
>
diff mbox

Patch

Save and restore EDGE_DFS_BACK in draw_cfg_edges

2017-07-03  Tom de Vries  <tom@codesourcery.com>

	* graph.c (draw_cfg_edges): Save and restore EDGE_DFS_BACK.

---
 gcc/graph.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/gcc/graph.c b/gcc/graph.c
index 9261732..628769b 100644
--- a/gcc/graph.c
+++ b/gcc/graph.c
@@ -243,19 +243,60 @@  draw_cfg_nodes (pretty_printer *pp, struct function *fun)
 }
 
 /* Draw all edges in the CFG.  Retreating edges are drawin as not
-   constraining, this makes the layout of the graph better.
-   (??? Calling mark_dfs_back may change the compiler's behavior when
-   dumping, but computing back edges here for ourselves is also not
-   desirable.)  */
+   constraining, this makes the layout of the graph better.  */
 
 static void
 draw_cfg_edges (pretty_printer *pp, struct function *fun)
 {
   basic_block bb;
+
+  /* Save EDGE_DFS_BACK flag to dfs_back.  */
+  auto_bitmap dfs_back;
+  edge e;
+  edge_iterator ei;
+  unsigned int idx = 0;
+  FOR_EACH_BB_FN (bb, cfun)
+    {
+      FOR_EACH_EDGE (e, ei, bb->preds)
+	{
+	  if (e->flags & EDGE_DFS_BACK)
+	    bitmap_set_bit (dfs_back, idx);
+	  idx++;
+	}
+      FOR_EACH_EDGE (e, ei, bb->succs)
+	{
+	  if (e->flags & EDGE_DFS_BACK)
+	    bitmap_set_bit (dfs_back, idx);
+	  idx++;
+	}
+    }
+
   mark_dfs_back_edges ();
   FOR_ALL_BB_FN (bb, cfun)
     draw_cfg_node_succ_edges (pp, fun->funcdef_no, bb);
 
+  /* Restore EDGE_DFS_BACK flag from dfs_back.  */
+  idx = 0;
+  FOR_EACH_BB_FN (bb, cfun)
+    {
+      FOR_EACH_EDGE (e, ei, bb->preds)
+	{
+	  if (bitmap_bit_p (dfs_back, idx))
+	    e->flags |= EDGE_DFS_BACK;
+	  else
+	    e->flags &= ~EDGE_DFS_BACK;
+	  idx++;
+	}
+      FOR_EACH_EDGE (e, ei, bb->succs)
+	{
+	  if (bitmap_bit_p (dfs_back, idx))
+	    e->flags |= EDGE_DFS_BACK;
+	  else
+	    e->flags &= ~EDGE_DFS_BACK;
+	  idx++;
+	}
+    }
+
   /* Add an invisible edge from ENTRY to EXIT, to improve the graph layout.  */
   pp_printf (pp,
 	     "\tfn_%d_basic_block_%d:s -> fn_%d_basic_block_%d:n "