[PATCH] ivopts costs debug

[PATCH] ivopts costs debug

[Segher Boessenkool]

Hi,

I've used this patch in the past for another port, and now again for
rs6000, and I think it is generally useful.  It prints very verbose
information to the dump file about how ivopts comes up with its costs
for various forms of memory accesses, which tends to show problems in
the target's address cost functions and the legitimize functions.

In this patch it is disabled by default -- it is very chatty.

It also shows that the LAST_VIRTUAL_REGISTER trickery ivopts does
does not work (legitimize_address can create new registers, so now
a) we have new registers anyway, and b) we use some for multiple
purposes.  Oops).

Is this okay for trunk?  Bootstrapped and tested on powerpc64-linux.


Segher


2015-08-18  Segher Boessenkool  <segher@kernel.crashing.org>

	* tree-ssa-loop-ivopts.c (IVOPTS_DEBUG_COSTS): New define.
	(get_address_cost): Add address cost debug code.

---
 gcc/tree-ssa-loop-ivopts.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 6bce3a1..ae29a6f 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -17,6 +17,9 @@ You should have received a copy of the GNU General Public License
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
+/* Define to 1 to debug how this pass comes up with its cost estimates.  */
+#define IVOPTS_DEBUG_COSTS 0
+
 /* This pass tries to find the optimal set of induction variables for the loop.
    It optimizes just the basic linear induction variables (although adding
    support for other types should not be too hard).  It includes the
@@ -3743,8 +3746,51 @@ get_address_cost (bool symbol_present, bool var_present,
 	  seq = get_insns ();
 	  end_sequence ();
 
-	  acost = seq_cost (seq, speed);
-	  acost += address_cost (addr, mem_mode, as, speed);
+	  unsigned acost1 = seq_cost (seq, speed);
+	  unsigned acost2 = address_cost (addr, mem_mode, as, speed);
+
+	  if (dump_file && IVOPTS_DEBUG_COSTS)
+	    {
+	      fprintf (dump_file, "======= sequence generated for  ");
+	      if (sym_p)
+		fprintf (dump_file, "sym + ");
+	      if (var_p)
+		fprintf (dump_file, "var + ");
+	      if (off_p)
+		fprintf (dump_file, "cst + ");
+	      if (rat_p)
+		fprintf (dump_file, "rat * ");
+	      fprintf (dump_file, "index:\n");
+
+	      print_rtl (dump_file, seq);
+
+	      fprintf (dump_file, "\n cost of seq is %u", acost1);
+
+	      if (seq && NEXT_INSN (seq))
+		{
+		  fprintf (dump_file, " (namely,");
+
+		  for (rtx_insn *insn = seq; insn; insn = NEXT_INSN (insn))
+		    {
+		      unsigned cost;
+		      rtx set = single_set (insn);
+		      if (set)
+			cost = set_rtx_cost (set, speed);
+		      else
+			cost = 1;
+		      fprintf (dump_file, " %u", cost);
+		    }
+
+		  fprintf (dump_file, ")");
+		}
+
+	      fprintf (dump_file, "\n\nremaining address is:\n");
+	      print_rtl_single (dump_file, addr);
+
+	      fprintf (dump_file, "\n cost of that address is %u\n\n", acost2);
+	    }
+
+	  acost = acost1 + acost2;
 
 	  if (!acost)
 	    acost = 1;
-- 
1.8.1.4

Re: [PATCH] ivopts costs debug

[Bin.Cheng]

On Wed, Aug 19, 2015 at 5:19 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> Hi,
>
> I've used this patch in the past for another port, and now again for
> rs6000, and I think it is generally useful.  It prints very verbose
> information to the dump file about how ivopts comes up with its costs
> for various forms of memory accesses, which tends to show problems in
> the target's address cost functions and the legitimize functions.
>
> In this patch it is disabled by default -- it is very chatty.

Hi,
I ran into back-end address cost issue before and this should be
useful in such cases.  Though there are a lot of dumps, it would be
better to classify it into existing dump option (TDF_DETAILS?) and
discard the use of macro.  Also the address cost will be tuned/dumped
later, we should differentiate between them by emphasizing this part
of dump is the original cost from back-end.
Maybe there will be some other comments.

>
> It also shows that the LAST_VIRTUAL_REGISTER trickery ivopts does
> does not work (legitimize_address can create new registers, so now
> a) we have new registers anyway, and b) we use some for multiple
> purposes.  Oops).
Yes, that makes seq dump a little weird.

Thanks,
bin
>
> Is this okay for trunk?  Bootstrapped and tested on powerpc64-linux.
>
>
> Segher
>
>
> 2015-08-18  Segher Boessenkool  <segher@kernel.crashing.org>
>
>         * tree-ssa-loop-ivopts.c (IVOPTS_DEBUG_COSTS): New define.
>         (get_address_cost): Add address cost debug code.
>
> ---
>  gcc/tree-ssa-loop-ivopts.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> index 6bce3a1..ae29a6f 100644
> --- a/gcc/tree-ssa-loop-ivopts.c
> +++ b/gcc/tree-ssa-loop-ivopts.c
> @@ -17,6 +17,9 @@ You should have received a copy of the GNU General Public License
>  along with GCC; see the file COPYING3.  If not see
>  <http://www.gnu.org/licenses/>.  */
>
> +/* Define to 1 to debug how this pass comes up with its cost estimates.  */
> +#define IVOPTS_DEBUG_COSTS 0
> +
>  /* This pass tries to find the optimal set of induction variables for the loop.
>     It optimizes just the basic linear induction variables (although adding
>     support for other types should not be too hard).  It includes the
> @@ -3743,8 +3746,51 @@ get_address_cost (bool symbol_present, bool var_present,
>           seq = get_insns ();
>           end_sequence ();
>
> -         acost = seq_cost (seq, speed);
> -         acost += address_cost (addr, mem_mode, as, speed);
> +         unsigned acost1 = seq_cost (seq, speed);
> +         unsigned acost2 = address_cost (addr, mem_mode, as, speed);
> +
> +         if (dump_file && IVOPTS_DEBUG_COSTS)
> +           {
> +             fprintf (dump_file, "======= sequence generated for  ");
> +             if (sym_p)
> +               fprintf (dump_file, "sym + ");
> +             if (var_p)
> +               fprintf (dump_file, "var + ");
> +             if (off_p)
> +               fprintf (dump_file, "cst + ");
> +             if (rat_p)
> +               fprintf (dump_file, "rat * ");
> +             fprintf (dump_file, "index:\n");
> +
> +             print_rtl (dump_file, seq);
> +
> +             fprintf (dump_file, "\n cost of seq is %u", acost1);
> +
> +             if (seq && NEXT_INSN (seq))
> +               {
> +                 fprintf (dump_file, " (namely,");
> +
> +                 for (rtx_insn *insn = seq; insn; insn = NEXT_INSN (insn))
> +                   {
> +                     unsigned cost;
> +                     rtx set = single_set (insn);
> +                     if (set)
> +                       cost = set_rtx_cost (set, speed);
> +                     else
> +                       cost = 1;
> +                     fprintf (dump_file, " %u", cost);
> +                   }
> +
> +                 fprintf (dump_file, ")");
> +               }
> +
> +             fprintf (dump_file, "\n\nremaining address is:\n");
> +             print_rtl_single (dump_file, addr);
> +
> +             fprintf (dump_file, "\n cost of that address is %u\n\n", acost2);
> +           }
> +
> +         acost = acost1 + acost2;
>
>           if (!acost)
>             acost = 1;
> --
> 1.8.1.4
>

Re: [PATCH] ivopts costs debug

[Segher Boessenkool]

On Wed, Aug 19, 2015 at 10:45:42AM +0800, Bin.Cheng wrote:
> I ran into back-end address cost issue before and this should be
> useful in such cases.  Though there are a lot of dumps, it would be
> better to classify it into existing dump option (TDF_DETAILS?) and
> discard the use of macro.

But TDF_DETAILS is enabled pretty much always, and this costs dump
isn't to debug ivopts _itself_.  I got the idea from lower-subreg.c,
which also uses debug macros like this (and is another place where
bad costs tend to show up, btw).

> Also the address cost will be tuned/dumped
> later, we should differentiate between them by emphasizing this part
> of dump is the original cost from back-end.

Yeah I should probably print some header, also say e.g. what machine
mode some table is for.

> > It also shows that the LAST_VIRTUAL_REGISTER trickery ivopts does
> > does not work (legitimize_address can create new registers, so now
> > a) we have new registers anyway, and b) we use some for multiple
> > purposes.  Oops).
> Yes, that makes seq dump a little weird.

It can even make the result wrong -- e.g. (plus (reg 155) (reg 155))
could  well be costed differently than (plus (reg 155) (reg 159)).


Segher