On Tue, 26 May 2020, David Malcolm wrote:
> On Mon, 2020-05-25 at 16:56 +0200, Richard Biener wrote:
> > This adds an alternate debug_dump_context similar to the one for
> > selftests but for interactive debugging routines. This allows
> > to share code between user-visible dumping via the dump_* API
> > and those debugging routines. The primary driver was SLP node
> > dumping which wasn't accessible from inside a gdb session up to
> > now.
> >
> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >
> > David, does this look OK?
>
> Overall, seems sane to me; a couple of items inline below.
>
> > Thanks,
> > Richard.
> >
> > 2020-05-25 Richard Biener <rguent...@suse.de>
> >
> > * dump-context.h (debug_dump_context): New class.
> > (dump_context): Make it friend.
> > * dumpfile.c (debug_dump_context::debug_dump_context):
> > Implement.
> > (debug_dump_context::~debug_dump_context): Likewise.
> > * tree-vect-slp.c: Include dump-context.h.
> > (vect_print_slp_tree): Dump a single SLP node.
> > (debug): New overload for slp_tree.
> > (vect_print_slp_graph): Rename from vect_print_slp_tree and
> > use that.
> > (vect_analyze_slp_instance): Adjust.
> > ---
> > gcc/dump-context.h | 19 ++++++++++++++++++
> > gcc/dumpfile.c | 26 +++++++++++++++++++++++++
> > gcc/tree-vect-slp.c | 47 +++++++++++++++++++++++++++++++++--------
> > ----
> > 3 files changed, 80 insertions(+), 12 deletions(-)
> >
> > diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> > index 347477f331e..6d51eaf31ad 100644
> > --- a/gcc/dump-context.h
> > +++ b/gcc/dump-context.h
> > @@ -29,6 +29,7 @@ along with GCC; see the file COPYING3. If not see
> >
> > class optrecord_json_writer;
> > namespace selftest { class temp_dump_context; }
> > +class debug_dump_context;
> >
> > /* A class for handling the various dump_* calls.
> >
> > @@ -42,6 +43,7 @@ namespace selftest { class temp_dump_context; }
> > class dump_context
> > {
> > friend class selftest::temp_dump_context;
> > + friend class debug_dump_context;
> >
> > public:
> > static dump_context &get () { return *s_current; }
> > @@ -195,6 +197,23 @@ private:
> > auto_vec<stashed_item> m_stashed_items;
> > };
> >
> > +/* An RAII-style class for use in debug dumpers for temporarily
> > using a
> > + different dump_context. */
> > +
> > +class debug_dump_context
>
> (Bikeshed Alert) The name might be confusing in that this class isn't
> a dump_context itself. Some of our existing RAII classes have an
> "auto_" prefix; would that be an idea?
> Maybe "auto_dump_everything"???
>
> But I don't have a strong objection to the name as-is.
kept it as-is but improved the class comment.
> [...snip...]
>
>
> > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> > index 54718784fd4..0c0c076d890 100644
> > --- a/gcc/dumpfile.c
> > +++ b/gcc/dumpfile.c
> > @@ -2078,6 +2078,32 @@ enable_rtl_dump_file (void)
> > return num_enabled > 0;
> > }
> >
> > +/* debug_dump_context's ctor. Temporarily override the dump_context
> > + (to forcibly enable output to stderr). */
> > +
> > +debug_dump_context::debug_dump_context ()
> > +: m_context (),
> > + m_saved (&dump_context::get ()),
> > + m_saved_flags (dump_flags),
> > + m_saved_file (dump_file)
> > +{
> > + set_dump_file (stderr);
> > + dump_context::s_current = &m_context;
> > + pflags = dump_flags = MSG_ALL_KINDS | MSG_ALL_PRIORITIES;
> > + dump_context::get ().refresh_dumps_are_enabled ();
> > +}
> > +
> > +/* debug_dump_context's dtor. Restore the saved dump_context. */
> > +
> > +debug_dump_context::~debug_dump_context ()
> > +{
> > + set_dump_file (m_saved_file);
> > + dump_context::s_current = m_saved;
> > + pflags = dump_flags = m_saved_flags;
> > + dump_context::get ().refresh_dumps_are_enabled ();
> > +}
>
> I notice that the code saves dump_flags, and later restores both
> dump_flags and pflags to the same value. I'm a little hazy on this,
> but is there any guarantee they had the same value? Should the value
> of pflags be saved separately from dump_flags?
Hum, right. Better be safe than sorry.
Re-testing the following, will commit after that succeeds.
Richard.
>From 73ddc25a76088571675aeccdd0537ebb2831b863 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Mon, 25 May 2020 16:10:12 +0200
Subject: [PATCH] Add debug (slp_tree) and dump infrastructure for this
This adds an alternate debug_dump_context similar to the one for
selftests but for interactive debugging routines. This allows
to share code between user-visible dumping via the dump_* API
and those debugging routines. The primary driver was SLP node
dumping which wasn't accessible from inside a gdb session up to
now.
2020-05-27 Richard Biener <rguent...@suse.de>
* dump-context.h (debug_dump_context): New class.
(dump_context): Make it friend.
* dumpfile.c (debug_dump_context::debug_dump_context):
Implement.
(debug_dump_context::~debug_dump_context): Likewise.
* tree-vect-slp.c: Include dump-context.h.
(vect_print_slp_tree): Dump a single SLP node.
(debug): New overload for slp_tree.
(vect_print_slp_graph): Rename from vect_print_slp_tree and
use that.
(vect_analyze_slp_instance): Adjust.
---
gcc/dump-context.h | 21 ++++++++++++++++++++
gcc/dumpfile.c | 28 +++++++++++++++++++++++++++
gcc/tree-vect-slp.c | 47 +++++++++++++++++++++++++++++++++------------
3 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/gcc/dump-context.h b/gcc/dump-context.h
index 347477f331e..3f72cc932a9 100644
--- a/gcc/dump-context.h
+++ b/gcc/dump-context.h
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3. If not see
class optrecord_json_writer;
namespace selftest { class temp_dump_context; }
+class debug_dump_context;
/* A class for handling the various dump_* calls.
@@ -42,6 +43,7 @@ namespace selftest { class temp_dump_context; }
class dump_context
{
friend class selftest::temp_dump_context;
+ friend class debug_dump_context;
public:
static dump_context &get () { return *s_current; }
@@ -195,6 +197,25 @@ private:
auto_vec<stashed_item> m_stashed_items;
};
+/* An RAII-style class for use in debug dumpers for temporarily using a
+ different dump_context. It enables full details and outputs to
+ stderr instead of the currently active dump_file. */
+
+class debug_dump_context
+{
+ public:
+ debug_dump_context ();
+ ~debug_dump_context ();
+
+ private:
+ dump_context m_context;
+ dump_context *m_saved;
+ dump_flags_t m_saved_flags;
+ dump_flags_t m_saved_pflags;
+ FILE *m_saved_file;
+};
+
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 54718784fd4..5d61946fc49 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -2078,6 +2078,34 @@ enable_rtl_dump_file (void)
return num_enabled > 0;
}
+/* debug_dump_context's ctor. Temporarily override the dump_context
+ (to forcibly enable output to stderr). */
+
+debug_dump_context::debug_dump_context ()
+: m_context (),
+ m_saved (&dump_context::get ()),
+ m_saved_flags (dump_flags),
+ m_saved_pflags (pflags),
+ m_saved_file (dump_file)
+{
+ set_dump_file (stderr);
+ dump_context::s_current = &m_context;
+ pflags = dump_flags = MSG_ALL_KINDS | MSG_ALL_PRIORITIES;
+ dump_context::get ().refresh_dumps_are_enabled ();
+}
+
+/* debug_dump_context's dtor. Restore the saved dump_context. */
+
+debug_dump_context::~debug_dump_context ()
+{
+ set_dump_file (m_saved_file);
+ dump_context::s_current = m_saved;
+ dump_flags = m_saved_flags;
+ pflags = m_saved_pflags;
+ dump_context::get ().refresh_dumps_are_enabled ();
+}
+
+
#if CHECKING_P
namespace selftest {
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index c4fd045e9be..c0c9afd0bd2 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see
#include "vec-perm-indices.h"
#include "gimple-fold.h"
#include "internal-fn.h"
+#include "dump-context.h"
/* Initialize a SLP node. */
@@ -1579,20 +1580,17 @@ fail:
return node;
}
-/* Dump a slp tree NODE using flags specified in DUMP_KIND. */
+/* Dump a single SLP tree NODE. */
static void
vect_print_slp_tree (dump_flags_t dump_kind, dump_location_t loc,
- slp_tree node, hash_set<slp_tree> &visited)
+ slp_tree node)
{
unsigned i, j;
- stmt_vec_info stmt_info;
slp_tree child;
+ stmt_vec_info stmt_info;
tree op;
- if (visited.add (node))
- return;
-
dump_metadata_t metadata (dump_kind, loc.get_impl_location ());
dump_user_location_t user_loc = loc.get_user_location ();
dump_printf_loc (metadata, user_loc, "node%s %p (max_nunits=%u,
refcnt=%u)\n",
@@ -1626,16 +1624,41 @@ vect_print_slp_tree (dump_flags_t dump_kind,
dump_location_t loc,
FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)
dump_printf (dump_kind, " %p", (void *)child);
dump_printf (dump_kind, "\n");
+}
+
+DEBUG_FUNCTION void
+debug (slp_tree node)
+{
+ debug_dump_context ctx;
+ vect_print_slp_tree (MSG_NOTE,
+ dump_location_t::from_location_t (UNKNOWN_LOCATION),
+ node);
+}
+
+/* Dump a slp tree NODE using flags specified in DUMP_KIND. */
+
+static void
+vect_print_slp_graph (dump_flags_t dump_kind, dump_location_t loc,
+ slp_tree node, hash_set<slp_tree> &visited)
+{
+ unsigned i;
+ slp_tree child;
+
+ if (visited.add (node))
+ return;
+
+ vect_print_slp_tree (dump_kind, loc, node);
+
FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)
- vect_print_slp_tree (dump_kind, loc, child, visited);
+ vect_print_slp_graph (dump_kind, loc, child, visited);
}
static void
-vect_print_slp_tree (dump_flags_t dump_kind, dump_location_t loc,
- slp_tree node)
+vect_print_slp_graph (dump_flags_t dump_kind, dump_location_t loc,
+ slp_tree entry)
{
hash_set<slp_tree> visited;
- vect_print_slp_tree (dump_kind, loc, node, visited);
+ vect_print_slp_graph (dump_kind, loc, entry, visited);
}
/* Mark the tree rooted at NODE with PURE_SLP. */
@@ -2240,8 +2263,8 @@ vect_analyze_slp_instance (vec_info *vinfo,
{
dump_printf_loc (MSG_NOTE, vect_location,
"Final SLP tree for instance:\n");
- vect_print_slp_tree (MSG_NOTE, vect_location,
- SLP_INSTANCE_TREE (new_instance));
+ vect_print_slp_graph (MSG_NOTE, vect_location,
+ SLP_INSTANCE_TREE (new_instance));
}
return true;
--
2.26.1
