Add debugging helpers for ranger.

Message ID 20210615114741.151342-1-aldyh@redhat.com
State New
Headers show
Series
  • Add debugging helpers for ranger.
Related show

Commit Message

Xionghu Luo via Gcc-patches June 15, 2021, 11:47 a.m.
These are aids to help in debugging ranger based passes.

I find the ability to dump the current ranger knowledge for the
function being debugged invaluable during development.  Similarly for
subsets of the CFG.

Tested with a bootstrap and regtest, as well as countless sessions in
gdb :).

OK?

gcc/ChangeLog:

	* gimple-range.cc (debug_seed_ranger): New.
	(dump_ranger): New.
	(debug_ranger): New.
---
 gcc/gimple-range.cc | 71 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

-- 
2.31.1

Comments

Xionghu Luo via Gcc-patches June 15, 2021, 11:55 a.m. | #1
On Tue, Jun 15, 2021 at 01:47:41PM +0200, Aldy Hernandez via Gcc-patches wrote:
> +// =========================================

> +// Debugging helpers.

> +// =========================================

> +

> +// Query all statements in the IL to precalculate computable ranges in RANGER.


Not a review, just a random nit.
The above comment doesn't match what the function is actually doing:

> +

> +static DEBUG_FUNCTION void

> +debug_seed_ranger (gimple_ranger &ranger)

> +{

> +  // Recalculate SCEV to make sure the dump lists everything.

> +  if (scev_initialized_p ())

> +    {

> +      scev_finalize ();

> +      scev_initialize ();

> +    }

> +

> +  basic_block bb;

> +  int_range_max r;

> +  FOR_EACH_BB_FN (bb, cfun)

> +    {

> +      gimple *last = last_stmt (bb);

> +      if (last && gimple_get_lhs (last))

> +	ranger.range_of_stmt (r, last);


which is only doing it for the last stmts in the basic blocks if any.
So e.g. in the common case of GIMPLE_COND at the end of a bb it does
nothing.

	Jakub
Xionghu Luo via Gcc-patches June 15, 2021, 1:43 p.m. | #2
On 6/15/21 7:55 AM, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Jun 15, 2021 at 01:47:41PM +0200, Aldy Hernandez via Gcc-patches wrote:

>> +// =========================================

>> +// Debugging helpers.

>> +// =========================================

>> +

>> +// Query all statements in the IL to precalculate computable ranges in RANGER.

> Not a review, just a random nit.

> The above comment doesn't match what the function is actually doing:

>

>> +

>> +static DEBUG_FUNCTION void

>> +debug_seed_ranger (gimple_ranger &ranger)

>> +{

>> +  // Recalculate SCEV to make sure the dump lists everything.

>> +  if (scev_initialized_p ())

>> +    {

>> +      scev_finalize ();

>> +      scev_initialize ();

>> +    }

>> +

>> +  basic_block bb;

>> +  int_range_max r;

>> +  FOR_EACH_BB_FN (bb, cfun)

>> +    {

>> +      gimple *last = last_stmt (bb);

>> +      if (last && gimple_get_lhs (last))

>> +	ranger.range_of_stmt (r, last);

> which is only doing it for the last stmts in the basic blocks if any.

> So e.g. in the common case of GIMPLE_COND at the end of a bb it does

> nothing.

>

> 	Jakub

>

In fact, you can simply drop the gimple_get_lhs (last) part of the 
condition... range_of_stmt works just fine without a LHS, and will then 
calculate the GIMPLE_COND operands.

Andrew
Xionghu Luo via Gcc-patches June 15, 2021, 1:48 p.m. | #3
On Tue, Jun 15, 2021 at 09:43:33AM -0400, Andrew MacLeod wrote:
> > > +  basic_block bb;

> > > +  int_range_max r;

> > > +  FOR_EACH_BB_FN (bb, cfun)

> > > +    {

> > > +      gimple *last = last_stmt (bb);

> > > +      if (last && gimple_get_lhs (last))

> > > +	ranger.range_of_stmt (r, last);

> > which is only doing it for the last stmts in the basic blocks if any.

> > So e.g. in the common case of GIMPLE_COND at the end of a bb it does

> > nothing.

> > 

> > 	Jakub

> > 

> In fact, you can simply drop the gimple_get_lhs (last) part of the

> condition... range_of_stmt works just fine without a LHS, and will then

> calculate the GIMPLE_COND operands.


But even then, not every stmt in the bb participates in the computation
of the value (if any) of the last stmt in the block, some stmts could
compute values only used in the PHIs or in the middle of other bbs,
others could store to memory, or be calls without return value, ...
If ranger does caching, can't it just call range_of_stmt on every non-debug
stmt in the bb?

	Jakub
Xionghu Luo via Gcc-patches June 15, 2021, 2:05 p.m. | #4
On 6/15/21 9:48 AM, Jakub Jelinek wrote:
> On Tue, Jun 15, 2021 at 09:43:33AM -0400, Andrew MacLeod wrote:

>>>> +  basic_block bb;

>>>> +  int_range_max r;

>>>> +  FOR_EACH_BB_FN (bb, cfun)

>>>> +    {

>>>> +      gimple *last = last_stmt (bb);

>>>> +      if (last && gimple_get_lhs (last))

>>>> +	ranger.range_of_stmt (r, last);

>>> which is only doing it for the last stmts in the basic blocks if any.

>>> So e.g. in the common case of GIMPLE_COND at the end of a bb it does

>>> nothing.

>>>

>>> 	Jakub

>>>

>> In fact, you can simply drop the gimple_get_lhs (last) part of the

>> condition... range_of_stmt works just fine without a LHS, and will then

>> calculate the GIMPLE_COND operands.

> But even then, not every stmt in the bb participates in the computation

> of the value (if any) of the last stmt in the block, some stmts could

> compute values only used in the PHIs or in the middle of other bbs,

> others could store to memory, or be calls without return value, ...

> If ranger does caching, can't it just call range_of_stmt on every non-debug

> stmt in the bb?

>

> 	Jakub

>

Yes.  Which is exactly what the old execute() routine use to do to fill 
the cache and dump state.

I suspect this is driven by his threader work, and in that particular 
case, all he cares about is things that are exported so he isn't 
noticing the other things in the middle of the block :-) Should have 
noticed the GIMPLE_CONDS I would have thought. perhaps they were 
collected indirectly if they were relevant.

Andrew
Xionghu Luo via Gcc-patches June 15, 2021, 2:26 p.m. | #5
On 6/15/21 4:05 PM, Andrew MacLeod wrote:
> On 6/15/21 9:48 AM, Jakub Jelinek wrote:

>> On Tue, Jun 15, 2021 at 09:43:33AM -0400, Andrew MacLeod wrote:

>>>>> +  basic_block bb;

>>>>> +  int_range_max r;

>>>>> +  FOR_EACH_BB_FN (bb, cfun)

>>>>> +    {

>>>>> +      gimple *last = last_stmt (bb);

>>>>> +      if (last && gimple_get_lhs (last))

>>>>> +    ranger.range_of_stmt (r, last);

>>>> which is only doing it for the last stmts in the basic blocks if any.

>>>> So e.g. in the common case of GIMPLE_COND at the end of a bb it does

>>>> nothing.

>>>>

>>>>     Jakub

>>>>

>>> In fact, you can simply drop the gimple_get_lhs (last) part of the

>>> condition... range_of_stmt works just fine without a LHS, and will then

>>> calculate the GIMPLE_COND operands.

>> But even then, not every stmt in the bb participates in the computation

>> of the value (if any) of the last stmt in the block, some stmts could

>> compute values only used in the PHIs or in the middle of other bbs,

>> others could store to memory, or be calls without return value, ...

>> If ranger does caching, can't it just call range_of_stmt on every 

>> non-debug

>> stmt in the bb?

>>

>>     Jakub

>>

> Yes.  Which is exactly what the old execute() routine use to do to fill 

> the cache and dump state.

> 

> I suspect this is driven by his threader work, and in that particular 

> case, all he cares about is things that are exported so he isn't 

> noticing the other things in the middle of the block :-) Should have 

> noticed the GIMPLE_CONDS I would have thought. perhaps they were 

> collected indirectly if they were relevant.


Bah.  Indeed.  Fixed.

Aldy
From c26af6145e2ed3b97a277dbbd506328ee7e01d47 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>

Date: Mon, 14 Jun 2021 11:39:33 +0200
Subject: [PATCH] Add debugging helpers for ranger.

These are debugging aids for help in debugging ranger based passes.

gcc/ChangeLog:

	* gimple-range.cc (debug_seed_ranger): New.
	(dump_ranger): New.
	(debug_ranger): New.
---
 gcc/gimple-range.cc | 76 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index b534b8e0a2c..5b92aa769a4 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1661,4 +1661,80 @@ disable_ranger (struct function *fun)
   fun->x_range_query = &global_ranges;
 }
 
+// =========================================
+// Debugging helpers.
+// =========================================
+
+// Query all statements in the IL to precalculate computable ranges in RANGER.
+
+static DEBUG_FUNCTION void
+debug_seed_ranger (gimple_ranger &ranger)
+{
+  // Recalculate SCEV to make sure the dump lists everything.
+  if (scev_initialized_p ())
+    {
+      scev_finalize ();
+      scev_initialize ();
+    }
+
+  basic_block bb;
+  int_range_max r;
+  gimple_stmt_iterator gsi;
+  FOR_EACH_BB_FN (bb, cfun)
+    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+      {
+	gimple *stmt = gsi_stmt (gsi);
+
+	if (is_gimple_debug (stmt))
+	  continue;
+
+	ranger.range_of_stmt (r, stmt);
+      }
+}
+
+// Dump all that ranger knows for the current function.
+
+DEBUG_FUNCTION void
+dump_ranger (FILE *out)
+{
+  gimple_ranger ranger;
+  debug_seed_ranger (ranger);
+  ranger.dump (out);
+}
+
+DEBUG_FUNCTION void
+debug_ranger ()
+{
+  dump_ranger (stderr);
+}
+
+// Dump all that ranger knows on a path of BBs.
+
+DEBUG_FUNCTION void
+dump_ranger (FILE *dump_file, const vec<basic_block> &path)
+{
+  if (path.length () == 0)
+    {
+      fprintf (dump_file, "empty\n");
+      return;
+    }
+
+  gimple_ranger ranger;
+  debug_seed_ranger (ranger);
+
+  unsigned i = path.length ();
+  do
+    {
+      i--;
+      ranger.dump_bb (dump_file, path[i]);
+    }
+  while (i > 0);
+}
+
+DEBUG_FUNCTION void
+debug_ranger (const vec<basic_block> &path)
+{
+  dump_ranger (stderr, path);
+}
+
 #include "gimple-range-tests.cc"
-- 
2.31.1
Xionghu Luo via Gcc-patches June 17, 2021, 8:32 a.m. | #6
Attached is the final version of the patch I have pushed.

Thanks.
Aldy

On Tue, Jun 15, 2021 at 4:26 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>

>

>

> On 6/15/21 4:05 PM, Andrew MacLeod wrote:

> > On 6/15/21 9:48 AM, Jakub Jelinek wrote:

> >> On Tue, Jun 15, 2021 at 09:43:33AM -0400, Andrew MacLeod wrote:

> >>>>> +  basic_block bb;

> >>>>> +  int_range_max r;

> >>>>> +  FOR_EACH_BB_FN (bb, cfun)

> >>>>> +    {

> >>>>> +      gimple *last = last_stmt (bb);

> >>>>> +      if (last && gimple_get_lhs (last))

> >>>>> +    ranger.range_of_stmt (r, last);

> >>>> which is only doing it for the last stmts in the basic blocks if any.

> >>>> So e.g. in the common case of GIMPLE_COND at the end of a bb it does

> >>>> nothing.

> >>>>

> >>>>     Jakub

> >>>>

> >>> In fact, you can simply drop the gimple_get_lhs (last) part of the

> >>> condition... range_of_stmt works just fine without a LHS, and will then

> >>> calculate the GIMPLE_COND operands.

> >> But even then, not every stmt in the bb participates in the computation

> >> of the value (if any) of the last stmt in the block, some stmts could

> >> compute values only used in the PHIs or in the middle of other bbs,

> >> others could store to memory, or be calls without return value, ...

> >> If ranger does caching, can't it just call range_of_stmt on every

> >> non-debug

> >> stmt in the bb?

> >>

> >>     Jakub

> >>

> > Yes.  Which is exactly what the old execute() routine use to do to fill

> > the cache and dump state.

> >

> > I suspect this is driven by his threader work, and in that particular

> > case, all he cares about is things that are exported so he isn't

> > noticing the other things in the middle of the block :-) Should have

> > noticed the GIMPLE_CONDS I would have thought. perhaps they were

> > collected indirectly if they were relevant.

>

> Bah.  Indeed.  Fixed.

>

> Aldy

>
From 3f3ee13959f852de432fa7761a8e50ddee6d6e1b Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Mon, 14 Jun 2021 11:39:33 +0200
Subject: [PATCH] Add debugging helpers for ranger.

These are debugging aids for help in debugging ranger based passes.

gcc/ChangeLog:

	* gimple-range.cc (debug_seed_ranger): New.
	(dump_ranger): New.
	(debug_ranger): New.
---
 gcc/gimple-range.cc | 79 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index efb919f1595..58109701f2f 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1662,4 +1662,83 @@ disable_ranger (struct function *fun)
   fun->x_range_query = &global_ranges;
 }
 
+// =========================================
+// Debugging helpers.
+// =========================================
+
+// Query all statements in the IL to precalculate computable ranges in RANGER.
+
+static DEBUG_FUNCTION void
+debug_seed_ranger (gimple_ranger &ranger)
+{
+  // Recalculate SCEV to make sure the dump lists everything.
+  if (scev_initialized_p ())
+    {
+      scev_finalize ();
+      scev_initialize ();
+    }
+
+  basic_block bb;
+  int_range_max r;
+  gimple_stmt_iterator gsi;
+  FOR_EACH_BB_FN (bb, cfun)
+    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+      {
+	gimple *stmt = gsi_stmt (gsi);
+
+	if (is_gimple_debug (stmt))
+	  continue;
+
+	ranger.range_of_stmt (r, stmt);
+      }
+}
+
+// Dump all that ranger knows for the current function.
+
+DEBUG_FUNCTION void
+dump_ranger (FILE *out)
+{
+  gimple_ranger ranger;
+  debug_seed_ranger (ranger);
+  ranger.dump (out);
+}
+
+DEBUG_FUNCTION void
+debug_ranger ()
+{
+  dump_ranger (stderr);
+}
+
+// Dump all that ranger knows on a path of BBs.
+//
+// Note that the blocks are in reverse order, thus the exit block is
+// path[0].
+
+DEBUG_FUNCTION void
+dump_ranger (FILE *dump_file, const vec<basic_block> &path)
+{
+  if (path.length () == 0)
+    {
+      fprintf (dump_file, "empty\n");
+      return;
+    }
+
+  gimple_ranger ranger;
+  debug_seed_ranger (ranger);
+
+  unsigned i = path.length ();
+  do
+    {
+      i--;
+      ranger.dump_bb (dump_file, path[i]);
+    }
+  while (i > 0);
+}
+
+DEBUG_FUNCTION void
+debug_ranger (const vec<basic_block> &path)
+{
+  dump_ranger (stderr, path);
+}
+
 #include "gimple-range-tests.cc"

Patch

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index b534b8e0a2c..2a0da417708 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1661,4 +1661,75 @@  disable_ranger (struct function *fun)
   fun->x_range_query = &global_ranges;
 }
 
+// =========================================
+// Debugging helpers.
+// =========================================
+
+// Query all statements in the IL to precalculate computable ranges in RANGER.
+
+static DEBUG_FUNCTION void
+debug_seed_ranger (gimple_ranger &ranger)
+{
+  // Recalculate SCEV to make sure the dump lists everything.
+  if (scev_initialized_p ())
+    {
+      scev_finalize ();
+      scev_initialize ();
+    }
+
+  basic_block bb;
+  int_range_max r;
+  FOR_EACH_BB_FN (bb, cfun)
+    {
+      gimple *last = last_stmt (bb);
+      if (last && gimple_get_lhs (last))
+	ranger.range_of_stmt (r, last);
+    }
+}
+
+// Dump all that ranger knows for the current function.
+
+DEBUG_FUNCTION void
+dump_ranger (FILE *out)
+{
+  gimple_ranger ranger;
+  debug_seed_ranger (ranger);
+  ranger.dump (out);
+}
+
+DEBUG_FUNCTION void
+debug_ranger ()
+{
+  dump_ranger (stderr);
+}
+
+// Dump all that ranger knows on a path of BBs.
+
+DEBUG_FUNCTION void
+dump_ranger (FILE *dump_file, const vec<basic_block> &path)
+{
+  if (path.length () == 0)
+    {
+      fprintf (dump_file, "empty\n");
+      return;
+    }
+
+  gimple_ranger ranger;
+  debug_seed_ranger (ranger);
+
+  unsigned i = path.length ();
+  do
+    {
+      i--;
+      ranger.dump_bb (dump_file, path[i]);
+    }
+  while (i > 0);
+}
+
+DEBUG_FUNCTION void
+debug_ranger (const vec<basic_block> &path)
+{
+  dump_ranger (stderr, path);
+}
+
 #include "gimple-range-tests.cc"