On Mon, 3 Jul 2017, Tom de Vries wrote:
> On 07/03/2017 11:53 AM, Richard Biener wrote:
> > On Mon, 3 Jul 2017, Tom de Vries wrote:
> >
> > > On 07/03/2017 09:05 AM, Richard Biener wrote:
> > > > On Mon, 3 Jul 2017, Tom de Vries wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > this patch adds a debug function dotfn and a convenience macro DOTFN
> > > > > similar
> > > > > to dot-fn in gdbhooks.py.
> > > > >
> > > > > It can be used to have the compiler:
> > > > > - dump a control flow graph, or
> > > > > - pop up a control flow graph window
> > > > > at specific moments in the compilation flow, for debugging purposes.
> > > > >
> > > > > Bootstrapped and reg-tested on x86_64.
> > > > >
> > > > > Used for debugging PR81192.
> > > > >
> > > > > OK for trunk?
> > > >
> > > > Why's dot-fn not enough? > I'd rather extend stuff in gdbhooks.py than
> > > > adding this kind of stuff to gcc itself.
> > >
> > > When expressing where and when to dump or pop-up a control flow graph,
> > > sometimes it's easier for me to do that in C than in gdb scripting.
> >
> > Ah, you mean by patching GCC. Yeah, I can see that this is useful
> > in some cases. OTOH I had dot-fn this way in my local dev tree for
> > a few years ...
> >
> > I'm retracting my objection but leave approval to somebody else
> > just to see if we can arrive at any consensus for "advanced"
> > debug stuff in GCC itself.
> >
>
> Ack.
>
> > For my usecase the gdb python stuff is now nearly perfect -- apart
> > from the cases where graph generation ICEs (like corrupt loop info).
>
> I suppose we can make a dotfn variant that calls draw_cfg_nodes_no_loops even
> if loop info is present.
Locally I have
@@ -236,7 +242,8 @@ draw_cfg_nodes_for_loop (pretty_printer
static void
draw_cfg_nodes (pretty_printer *pp, struct function *fun)
{
- if (loops_for_fn (fun))
+ if (loops_for_fn (fun)
+ && !(loops_for_fn (fun)->state & LOOPS_NEED_FIXUP))
draw_cfg_nodes_for_loop (pp, fun->funcdef_no, get_loop (fun, 0));
else
draw_cfg_nodes_no_loops (pp, fun);
that avoids most of the cases but of course not always. I suppose
a special dump_flag might work here. The problem is really
get_loop_body* trusting loop->num_nodes and ICEing when that doesn't
match. Using get_loop_body_with_size with n_basic_blocks_for_fn
would avoid that but it isn't a replacement for
get_loop_body_in_bfs_order -- at least with get_loop_body_with_size
we could avoid repeatedly allocating the array in draw_cfg_nodes_for_loop.
Not sure if bfs_order dots so much nicer than dfs order.
>
> 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.
> Thanks,
> - Tom
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
