[PATCH] Make vectorizer dumps more comparable

[PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

This avoids the file/location clutter in front of each line
in the vectorizer dump.  While this is useful for people
requesting -fvectorizer-verbose=N in dump files this makes
you unable to compare dumps for testcases on a branch and trunk.
It also makes lines excessively long because the testsuite
filename paths are so long.  Very annoying.

(I'd argue also that -fvectorizer-verbose=N dumps to the dump
file if available and not always to stderr is bogus, but well ...)

This patch has made my life a lot easier debugging the data dependence 
stuff.

Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on trunk.

Richard.

2011-08-24  Richard Guenther  <rguenther@suse.de>

	* tree-vectorizer.c (vect_print_dump_info): Avoid the
	file and location clutter when dumping to dump files.

Index: gcc/tree-vectorizer.c
===================================================================
--- gcc/tree-vectorizer.c	(revision 178028)
+++ gcc/tree-vectorizer.c	(working copy)
@@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
   if (!current_function_decl || !vect_dump)
     return false;
 
-  if (vect_location == UNKNOWN_LOC)
+  if (dump_file)
+    fprintf (vect_dump, "\n");
+
+  else if (vect_location == UNKNOWN_LOC)
     fprintf (vect_dump, "\n%s:%d: note: ",
 	     DECL_SOURCE_FILE (current_function_decl),
 	     DECL_SOURCE_LINE (current_function_decl));

Re: [PATCH] Make vectorizer dumps more comparable

[Michael Matz]

Hi,

On Wed, 24 Aug 2011, Richard Guenther wrote:

> This avoids the file/location clutter in front of each line in the 
> vectorizer dump.  While this is useful for people requesting 
> -fvectorizer-verbose=N in dump files this makes you unable to compare 
> dumps for testcases on a branch and trunk. It also makes lines 
> excessively long because the testsuite filename paths are so long.  
> Very annoying.

While the repetition is indeed annoying I put the location marker to good 
use for files with multiple loops.  Just searching for "123:" jumped you 
to the dump block where the interesting loop started to be analyzed.  Same 
for searching from the end, where it put you right at the final decision.

I would very much prefer if that would still be possible, possibly by 
using "start to analyze loop at file:123" and "finished with loop at 
file:123".


Ciao,
Michael.

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

On Wed, 24 Aug 2011, Michael Matz wrote:

> Hi,
> 
> On Wed, 24 Aug 2011, Richard Guenther wrote:
> 
> > This avoids the file/location clutter in front of each line in the 
> > vectorizer dump.  While this is useful for people requesting 
> > -fvectorizer-verbose=N in dump files this makes you unable to compare 
> > dumps for testcases on a branch and trunk. It also makes lines 
> > excessively long because the testsuite filename paths are so long.  
> > Very annoying.
> 
> While the repetition is indeed annoying I put the location marker to good 
> use for files with multiple loops.  Just searching for "123:" jumped you 
> to the dump block where the interesting loop started to be analyzed.  Same 
> for searching from the end, where it put you right at the final decision.
> 
> I would very much prefer if that would still be possible, possibly by 
> using "start to analyze loop at file:123" and "finished with loop at 
> file:123".

Yes, that would be nice.  Patches welcome ;)

Richard.

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

gcc-patches-owner@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
>
> This avoids the file/location clutter in front of each line
> in the vectorizer dump.  While this is useful for people
> requesting -fvectorizer-verbose=N in dump files this makes
> you unable to compare dumps for testcases on a branch and trunk.
> It also makes lines excessively long because the testsuite
> filename paths are so long.  Very annoying.
>
> (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> file if available and not always to stderr is bogus, but well ...)
>
> This patch has made my life a lot easier debugging the data dependence
> stuff.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on trunk.
>
> Richard.
>
> 2011-08-24  Richard Guenther  <rguenther@suse.de>
>
>    * tree-vectorizer.c (vect_print_dump_info): Avoid the
>    file and location clutter when dumping to dump files.


IMO it's a bad idea. It's now impossible to find anything when compiling a
big file. How about only removing the file name?

Index: tree-vectorizer.c
===================================================================
--- tree-vectorizer.c   (revision 178374)
+++ tree-vectorizer.c   (working copy)
@@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
   if (!current_function_decl || !vect_dump)
     return false;

-  if (dump_file)
-    fprintf (vect_dump, "\n");
-
-  else if (vect_location == UNKNOWN_LOC)
-    fprintf (vect_dump, "\n%s:%d: note: ",
-            DECL_SOURCE_FILE (current_function_decl),
+  if (vect_location == UNKNOWN_LOC)
+    fprintf (vect_dump, "\nline %d: ",
             DECL_SOURCE_LINE (current_function_decl));
   else
-    fprintf (vect_dump, "\n%s:%d: note: ",
-            LOC_FILE (vect_location), LOC_LINE (vect_location));
+    fprintf (vect_dump, "\nline %d: ",
+            LOC_LINE (vect_location));

   return true;
 }

Ira

>
> Index: gcc/tree-vectorizer.c
> ===================================================================
> --- gcc/tree-vectorizer.c   (revision 178028)
> +++ gcc/tree-vectorizer.c   (working copy)
> @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
>    if (!current_function_decl || !vect_dump)
>      return false;
>
> -  if (vect_location == UNKNOWN_LOC)
> +  if (dump_file)
> +    fprintf (vect_dump, "\n");
> +
> +  else if (vect_location == UNKNOWN_LOC)
>      fprintf (vect_dump, "\n%s:%d: note: ",
>          DECL_SOURCE_FILE (current_function_decl),
>          DECL_SOURCE_LINE (current_function_decl));
>

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3044 bytes --]

On Thu, 1 Sep 2011, Ira Rosen wrote:

> 
> 
> gcc-patches-owner@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
> >
> > This avoids the file/location clutter in front of each line
> > in the vectorizer dump.  While this is useful for people
> > requesting -fvectorizer-verbose=N in dump files this makes
> > you unable to compare dumps for testcases on a branch and trunk.
> > It also makes lines excessively long because the testsuite
> > filename paths are so long.  Very annoying.
> >
> > (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> > file if available and not always to stderr is bogus, but well ...)
> >
> > This patch has made my life a lot easier debugging the data dependence
> > stuff.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on trunk.
> >
> > Richard.
> >
> > 2011-08-24  Richard Guenther  <rguenther@suse.de>
> >
> >    * tree-vectorizer.c (vect_print_dump_info): Avoid the
> >    file and location clutter when dumping to dump files.
> 
> 
> IMO it's a bad idea. It's now impossible to find anything when compiling a
> big file. How about only removing the file name?

How about, as Micha suggested, print the location of the loop
we currently investigate from vectorize_loops () where we
call find_loop_location () instead?

Richard.

> Index: tree-vectorizer.c
> ===================================================================
> --- tree-vectorizer.c   (revision 178374)
> +++ tree-vectorizer.c   (working copy)
> @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
>    if (!current_function_decl || !vect_dump)
>      return false;
> 
> -  if (dump_file)
> -    fprintf (vect_dump, "\n");
> -
> -  else if (vect_location == UNKNOWN_LOC)
> -    fprintf (vect_dump, "\n%s:%d: note: ",
> -            DECL_SOURCE_FILE (current_function_decl),
> +  if (vect_location == UNKNOWN_LOC)
> +    fprintf (vect_dump, "\nline %d: ",
>              DECL_SOURCE_LINE (current_function_decl));
>    else
> -    fprintf (vect_dump, "\n%s:%d: note: ",
> -            LOC_FILE (vect_location), LOC_LINE (vect_location));
> +    fprintf (vect_dump, "\nline %d: ",
> +            LOC_LINE (vect_location));
> 
>    return true;
>  }
> 
> Ira
> 
> >
> > Index: gcc/tree-vectorizer.c
> > ===================================================================
> > --- gcc/tree-vectorizer.c   (revision 178028)
> > +++ gcc/tree-vectorizer.c   (working copy)
> > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> >    if (!current_function_decl || !vect_dump)
> >      return false;
> >
> > -  if (vect_location == UNKNOWN_LOC)
> > +  if (dump_file)
> > +    fprintf (vect_dump, "\n");
> > +
> > +  else if (vect_location == UNKNOWN_LOC)
> >      fprintf (vect_dump, "\n%s:%d: note: ",
> >          DECL_SOURCE_FILE (current_function_decl),
> >          DECL_SOURCE_LINE (current_function_decl));
> >
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

Richard Guenther <rguenther@suse.de> wrote on 01/09/2011 10:33:23 AM:

> On Thu, 1 Sep 2011, Ira Rosen wrote:
>
> >
> >
> > gcc-patches-owner@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
> > >
> > > This avoids the file/location clutter in front of each line
> > > in the vectorizer dump.  While this is useful for people
> > > requesting -fvectorizer-verbose=N in dump files this makes
> > > you unable to compare dumps for testcases on a branch and trunk.
> > > It also makes lines excessively long because the testsuite
> > > filename paths are so long.  Very annoying.
> > >
> > > (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> > > file if available and not always to stderr is bogus, but well ...)
> > >
> > > This patch has made my life a lot easier debugging the data
dependence
> > > stuff.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on
trunk.
> > >
> > > Richard.
> > >
> > > 2011-08-24  Richard Guenther  <rguenther@suse.de>
> > >
> > >    * tree-vectorizer.c (vect_print_dump_info): Avoid the
> > >    file and location clutter when dumping to dump files.
> >
> >
> > IMO it's a bad idea. It's now impossible to find anything when
compiling a
> > big file. How about only removing the file name?
>
> How about, as Micha suggested, print the location of the loop
> we currently investigate from vectorize_loops () where we
> call find_loop_location () instead?

The problem is that a dump of a single loop can be pretty long, and "start
to analyze loop..."/"finish to analyze loop..." may be not visible enough.
I am OK with adding these printings though (in addition to line numbers).

I understand why you didn't like to see the file location, but what's the
problem with the line number?

Ira

>
> Richard.
>
> > Index: tree-vectorizer.c
> > ===================================================================
> > --- tree-vectorizer.c   (revision 178374)
> > +++ tree-vectorizer.c   (working copy)
> > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> >    if (!current_function_decl || !vect_dump)
> >      return false;
> >
> > -  if (dump_file)
> > -    fprintf (vect_dump, "\n");
> > -
> > -  else if (vect_location == UNKNOWN_LOC)
> > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > -            DECL_SOURCE_FILE (current_function_decl),
> > +  if (vect_location == UNKNOWN_LOC)
> > +    fprintf (vect_dump, "\nline %d: ",
> >              DECL_SOURCE_LINE (current_function_decl));
> >    else
> > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > -            LOC_FILE (vect_location), LOC_LINE (vect_location));
> > +    fprintf (vect_dump, "\nline %d: ",
> > +            LOC_LINE (vect_location));
> >
> >    return true;
> >  }
> >
> > Ira
> >
> > >
> > > Index: gcc/tree-vectorizer.c
> > > ===================================================================
> > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > +++ gcc/tree-vectorizer.c   (working copy)
> > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > >    if (!current_function_decl || !vect_dump)
> > >      return false;
> > >
> > > -  if (vect_location == UNKNOWN_LOC)
> > > +  if (dump_file)
> > > +    fprintf (vect_dump, "\n");
> > > +
> > > +  else if (vect_location == UNKNOWN_LOC)
> > >      fprintf (vect_dump, "\n%s:%d: note: ",
> > >          DECL_SOURCE_FILE (current_function_decl),
> > >          DECL_SOURCE_LINE (current_function_decl));
> > >
> >
> >
>
> --
> Richard Guenther <rguenther@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Sandiford]

Ira Rosen <IRAR@il.ibm.com> writes:
>> How about, as Micha suggested, print the location of the loop
>> we currently investigate from vectorize_loops () where we
>> call find_loop_location () instead?
>
> The problem is that a dump of a single loop can be pretty long, and "start
> to analyze loop..."/"finish to analyze loop..." may be not visible enough.
> I am OK with adding these printings though (in addition to line numbers).
>
> I understand why you didn't like to see the file location, but what's the
> problem with the line number?

+1 FWIW.  I found these per-line locations really useful when doing
the strided load/store stuff.

Richard

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4783 bytes --]

On Thu, 1 Sep 2011, Ira Rosen wrote:

> 
> 
> Richard Guenther <rguenther@suse.de> wrote on 01/09/2011 10:33:23 AM:
> 
> > On Thu, 1 Sep 2011, Ira Rosen wrote:
> >
> > >
> > >
> > > gcc-patches-owner@gcc.gnu.org wrote on 24/08/2011 02:20:50 PM:
> > > >
> > > > This avoids the file/location clutter in front of each line
> > > > in the vectorizer dump.  While this is useful for people
> > > > requesting -fvectorizer-verbose=N in dump files this makes
> > > > you unable to compare dumps for testcases on a branch and trunk.
> > > > It also makes lines excessively long because the testsuite
> > > > filename paths are so long.  Very annoying.
> > > >
> > > > (I'd argue also that -fvectorizer-verbose=N dumps to the dump
> > > > file if available and not always to stderr is bogus, but well ...)
> > > >
> > > > This patch has made my life a lot easier debugging the data
> dependence
> > > > stuff.
> > > >
> > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, installed on
> trunk.
> > > >
> > > > Richard.
> > > >
> > > > 2011-08-24  Richard Guenther  <rguenther@suse.de>
> > > >
> > > >    * tree-vectorizer.c (vect_print_dump_info): Avoid the
> > > >    file and location clutter when dumping to dump files.
> > >
> > >
> > > IMO it's a bad idea. It's now impossible to find anything when
> compiling a
> > > big file. How about only removing the file name?
> >
> > How about, as Micha suggested, print the location of the loop
> > we currently investigate from vectorize_loops () where we
> > call find_loop_location () instead?
> 
> The problem is that a dump of a single loop can be pretty long, and "start
> to analyze loop..."/"finish to analyze loop..." may be not visible enough.
> I am OK with adding these printings though (in addition to line numbers).
> 
> I understand why you didn't like to see the file location, but what's the
> problem with the line number?

Well, it seems to be different what everybody else does and it's
highly redundant for a whole bunch of lines.

But, it solves my diff issue and the overly long lines as well.

Your patch changes both dump-file and stderr printing though,
I did want to preserve stderr printing.

For the dump-file I'd drop the 'line ' prefix and just print '%d: '.

Btw, the diagnostic machinery does _not_ print locations
for note (""), the location information is supposed to be printed
in the heading warning/error.  Thus, a much better format for stderr
would be

file.c:12: LOOP NOT VECTORIZED
note: unsupported stmt '....'

as the further notes will be printed with the 'loop location' which
is confusing when dumping statements

Richard.

> Ira
> 
> >
> > Richard.
> >
> > > Index: tree-vectorizer.c
> > > ===================================================================
> > > --- tree-vectorizer.c   (revision 178374)
> > > +++ tree-vectorizer.c   (working copy)
> > > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> > >    if (!current_function_decl || !vect_dump)
> > >      return false;
> > >
> > > -  if (dump_file)
> > > -    fprintf (vect_dump, "\n");
> > > -
> > > -  else if (vect_location == UNKNOWN_LOC)
> > > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > > -            DECL_SOURCE_FILE (current_function_decl),
> > > +  if (vect_location == UNKNOWN_LOC)
> > > +    fprintf (vect_dump, "\nline %d: ",
> > >              DECL_SOURCE_LINE (current_function_decl));
> > >    else
> > > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > > -            LOC_FILE (vect_location), LOC_LINE (vect_location));
> > > +    fprintf (vect_dump, "\nline %d: ",
> > > +            LOC_LINE (vect_location));
> > >
> > >    return true;
> > >  }
> > >
> > > Ira
> > >
> > > >
> > > > Index: gcc/tree-vectorizer.c
> > > > ===================================================================
> > > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > > +++ gcc/tree-vectorizer.c   (working copy)
> > > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > > >    if (!current_function_decl || !vect_dump)
> > > >      return false;
> > > >
> > > > -  if (vect_location == UNKNOWN_LOC)
> > > > +  if (dump_file)
> > > > +    fprintf (vect_dump, "\n");
> > > > +
> > > > +  else if (vect_location == UNKNOWN_LOC)
> > > >      fprintf (vect_dump, "\n%s:%d: note: ",
> > > >          DECL_SOURCE_FILE (current_function_decl),
> > > >          DECL_SOURCE_LINE (current_function_decl));
> > > >
> > >
> > >
> >
> > --
> > Richard Guenther <rguenther@suse.de>
> > SUSE / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

Richard Guenther <rguenther@suse.de> wrote on 01/09/2011 11:13:29 AM:

> > > > IMO it's a bad idea. It's now impossible to find anything when
> > compiling a
> > > > big file. How about only removing the file name?
> > >
> > > How about, as Micha suggested, print the location of the loop
> > > we currently investigate from vectorize_loops () where we
> > > call find_loop_location () instead?
> >
> > The problem is that a dump of a single loop can be pretty long, and
"start
> > to analyze loop..."/"finish to analyze loop..." may be not visible
enough.
> > I am OK with adding these printings though (in addition to line
numbers).
> >
> > I understand why you didn't like to see the file location, but what's
the
> > problem with the line number?
>
> Well, it seems to be different what everybody else does and it's
> highly redundant for a whole bunch of lines.
>
> But, it solves my diff issue and the overly long lines as well.
>
> Your patch changes both dump-file and stderr printing though,
> I did want to preserve stderr printing.

OK.

>
> For the dump-file I'd drop the 'line ' prefix and just print '%d: '.

OK.

>
> Btw, the diagnostic machinery does _not_ print locations
> for note (""), the location information is supposed to be printed
> in the heading warning/error.  Thus, a much better format for stderr
> would be
>
> file.c:12: LOOP NOT VECTORIZED
> note: unsupported stmt '....'
>
> as the further notes will be printed with the 'loop location' which
> is confusing when dumping statements

We usually print only one line, like

file.c:12: note: <message> <stmt>

so I don't really understand this part.

Ira

>
> Richard.
>
> > Ira
> >
> > >
> > > Richard.
> > >
> > > > Index: tree-vectorizer.c
> > > > ===================================================================
> > > > --- tree-vectorizer.c   (revision 178374)
> > > > +++ tree-vectorizer.c   (working copy)
> > > > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> > > >    if (!current_function_decl || !vect_dump)
> > > >      return false;
> > > >
> > > > -  if (dump_file)
> > > > -    fprintf (vect_dump, "\n");
> > > > -
> > > > -  else if (vect_location == UNKNOWN_LOC)
> > > > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > > > -            DECL_SOURCE_FILE (current_function_decl),
> > > > +  if (vect_location == UNKNOWN_LOC)
> > > > +    fprintf (vect_dump, "\nline %d: ",
> > > >              DECL_SOURCE_LINE (current_function_decl));
> > > >    else
> > > > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > > > -            LOC_FILE (vect_location), LOC_LINE (vect_location));
> > > > +    fprintf (vect_dump, "\nline %d: ",
> > > > +            LOC_LINE (vect_location));
> > > >
> > > >    return true;
> > > >  }
> > > >
> > > > Ira
> > > >
> > > > >
> > > > > Index: gcc/tree-vectorizer.c
> > > > >
===================================================================
> > > > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > > > +++ gcc/tree-vectorizer.c   (working copy)
> > > > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > > > >    if (!current_function_decl || !vect_dump)
> > > > >      return false;
> > > > >
> > > > > -  if (vect_location == UNKNOWN_LOC)
> > > > > +  if (dump_file)
> > > > > +    fprintf (vect_dump, "\n");
> > > > > +
> > > > > +  else if (vect_location == UNKNOWN_LOC)
> > > > >      fprintf (vect_dump, "\n%s:%d: note: ",
> > > > >          DECL_SOURCE_FILE (current_function_decl),
> > > > >          DECL_SOURCE_LINE (current_function_decl));
> > > > >
> > > >
> > > >
> > >
> > > --
> > > Richard Guenther <rguenther@suse.de>
> > > SUSE / SUSE Labs
> > > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> >
> >
>
> --
> Richard Guenther <rguenther@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4752 bytes --]

On Thu, 1 Sep 2011, Ira Rosen wrote:

> 
> 
> Richard Guenther <rguenther@suse.de> wrote on 01/09/2011 11:13:29 AM:
> 
> > > > > IMO it's a bad idea. It's now impossible to find anything when
> > > compiling a
> > > > > big file. How about only removing the file name?
> > > >
> > > > How about, as Micha suggested, print the location of the loop
> > > > we currently investigate from vectorize_loops () where we
> > > > call find_loop_location () instead?
> > >
> > > The problem is that a dump of a single loop can be pretty long, and
> "start
> > > to analyze loop..."/"finish to analyze loop..." may be not visible
> enough.
> > > I am OK with adding these printings though (in addition to line
> numbers).
> > >
> > > I understand why you didn't like to see the file location, but what's
> the
> > > problem with the line number?
> >
> > Well, it seems to be different what everybody else does and it's
> > highly redundant for a whole bunch of lines.
> >
> > But, it solves my diff issue and the overly long lines as well.
> >
> > Your patch changes both dump-file and stderr printing though,
> > I did want to preserve stderr printing.
> 
> OK.
> 
> >
> > For the dump-file I'd drop the 'line ' prefix and just print '%d: '.
> 
> OK.
> 
> >
> > Btw, the diagnostic machinery does _not_ print locations
> > for note (""), the location information is supposed to be printed
> > in the heading warning/error.  Thus, a much better format for stderr
> > would be
> >
> > file.c:12: LOOP NOT VECTORIZED
> > note: unsupported stmt '....'
> >
> > as the further notes will be printed with the 'loop location' which
> > is confusing when dumping statements
> 
> We usually print only one line, like
> 
> file.c:12: note: <message> <stmt>
> 
> so I don't really understand this part.

It's a general note.  With -ftree-vectorizer-verbose=5 we dump a lot
of information with the same location (that of the loop header),
but the individual messages refer not only to the overall loop
but to specific statements, etc.

This all is of course an artifact of sharing the dump file code
with the reporting code.

Richard.

> Ira
> 
> >
> > Richard.
> >
> > > Ira
> > >
> > > >
> > > > Richard.
> > > >
> > > > > Index: tree-vectorizer.c
> > > > > ===================================================================
> > > > > --- tree-vectorizer.c   (revision 178374)
> > > > > +++ tree-vectorizer.c   (working copy)
> > > > > @@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosit
> > > > >    if (!current_function_decl || !vect_dump)
> > > > >      return false;
> > > > >
> > > > > -  if (dump_file)
> > > > > -    fprintf (vect_dump, "\n");
> > > > > -
> > > > > -  else if (vect_location == UNKNOWN_LOC)
> > > > > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > > > > -            DECL_SOURCE_FILE (current_function_decl),
> > > > > +  if (vect_location == UNKNOWN_LOC)
> > > > > +    fprintf (vect_dump, "\nline %d: ",
> > > > >              DECL_SOURCE_LINE (current_function_decl));
> > > > >    else
> > > > > -    fprintf (vect_dump, "\n%s:%d: note: ",
> > > > > -            LOC_FILE (vect_location), LOC_LINE (vect_location));
> > > > > +    fprintf (vect_dump, "\nline %d: ",
> > > > > +            LOC_LINE (vect_location));
> > > > >
> > > > >    return true;
> > > > >  }
> > > > >
> > > > > Ira
> > > > >
> > > > > >
> > > > > > Index: gcc/tree-vectorizer.c
> > > > > >
> ===================================================================
> > > > > > --- gcc/tree-vectorizer.c   (revision 178028)
> > > > > > +++ gcc/tree-vectorizer.c   (working copy)
> > > > > > @@ -149,7 +149,10 @@ vect_print_dump_info (enum vect_verbosit
> > > > > >    if (!current_function_decl || !vect_dump)
> > > > > >      return false;
> > > > > >
> > > > > > -  if (vect_location == UNKNOWN_LOC)
> > > > > > +  if (dump_file)
> > > > > > +    fprintf (vect_dump, "\n");
> > > > > > +
> > > > > > +  else if (vect_location == UNKNOWN_LOC)
> > > > > >      fprintf (vect_dump, "\n%s:%d: note: ",
> > > > > >          DECL_SOURCE_FILE (current_function_decl),
> > > > > >          DECL_SOURCE_LINE (current_function_decl));
> > > > > >
> > > > >
> > > > >
> > > >
> > > > --
> > > > Richard Guenther <rguenther@suse.de>
> > > > SUSE / SUSE Labs
> > > > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> > >
> > >
> >
> > --
> > Richard Guenther <rguenther@suse.de>
> > SUSE / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] Make vectorizer dumps more comparable

[Ira Rosen]

Richard Guenther <rguenther@suse.de> wrote on 01/09/2011 12:26:25 PM:

> > > Well, it seems to be different what everybody else does and it's
> > > highly redundant for a whole bunch of lines.
> > >
> > > But, it solves my diff issue and the overly long lines as well.
> > >
> > > Your patch changes both dump-file and stderr printing though,
> > > I did want to preserve stderr printing.
> >
> > OK.
> >
> > >
> > > For the dump-file I'd drop the 'line ' prefix and just print '%d: '.
> >
> > OK.
> >
> > >
> > > Btw, the diagnostic machinery does _not_ print locations
> > > for note (""), the location information is supposed to be printed
> > > in the heading warning/error.  Thus, a much better format for stderr
> > > would be
> > >
> > > file.c:12: LOOP NOT VECTORIZED
> > > note: unsupported stmt '....'
> > >
> > > as the further notes will be printed with the 'loop location' which
> > > is confusing when dumping statements
> >
> > We usually print only one line, like
> >
> > file.c:12: note: <message> <stmt>
> >
> > so I don't really understand this part.
>
> It's a general note.  With -ftree-vectorizer-verbose=5 we dump a lot
> of information with the same location (that of the loop header),
> but the individual messages refer not only to the overall loop
> but to specific statements, etc.
>
> This all is of course an artifact of sharing the dump file code
> with the reporting code.

I see.

Here is the new patch, I'll commit it after testing (on
powerpc64-suse-linux) if there are no objections.

Ira


ChangeLog:

	* tree-vectorizer.c (vect_print_dump_info): Print line
	number when dumping to a file.
	(vectorize_loops): Add new messages to dump file.


Index: tree-vectorizer.c
===================================================================
--- tree-vectorizer.c   (revision 178396)
+++ tree-vectorizer.c   (working copy)
@@ -149,16 +149,12 @@ vect_print_dump_info (enum vect_verbosity_levels v
   if (!current_function_decl || !vect_dump)
     return false;

-  if (dump_file)
-    fprintf (vect_dump, "\n");
-
-  else if (vect_location == UNKNOWN_LOC)
+  if (vect_location == UNKNOWN_LOC)
     fprintf (vect_dump, "\n%s:%d: note: ",
             DECL_SOURCE_FILE (current_function_decl),
             DECL_SOURCE_LINE (current_function_decl));
   else
-    fprintf (vect_dump, "\n%s:%d: note: ",
-            LOC_FILE (vect_location), LOC_LINE (vect_location));
+    fprintf (vect_dump, "\n%d: ", LOC_LINE (vect_location));

   return true;
 }
@@ -199,12 +195,22 @@ vectorize_loops (void)
        loop_vec_info loop_vinfo;

        vect_location = find_loop_location (loop);
+        if (vect_location != UNKNOWN_LOC
+            && vect_verbosity_level > REPORT_NONE)
+         fprintf (vect_dump, "\nAnalyzing loop at %s:%d\n",
+            LOC_FILE (vect_location), LOC_LINE (vect_location));
+
        loop_vinfo = vect_analyze_loop (loop);
        loop->aux = loop_vinfo;

        if (!loop_vinfo || !LOOP_VINFO_VECTORIZABLE_P (loop_vinfo))
          continue;

+        if (vect_location != UNKNOWN_LOC
+            && vect_verbosity_level > REPORT_NONE)
+          fprintf (vect_dump, "\n\nVectorizing loop at %s:%d\n",
+            LOC_FILE (vect_location), LOC_LINE (vect_location));
+
        vect_transform_loop (loop_vinfo);
        num_vectorized_loops++;
       }

Re: [PATCH] Make vectorizer dumps more comparable

[Michael Matz]

Hi,

On Thu, 1 Sep 2011, Richard Sandiford wrote:

> Ira Rosen <IRAR@il.ibm.com> writes:
> >> How about, as Micha suggested, print the location of the loop
> >> we currently investigate from vectorize_loops () where we
> >> call find_loop_location () instead?
> >
> > The problem is that a dump of a single loop can be pretty long, and "start
> > to analyze loop..."/"finish to analyze loop..." may be not visible enough.
> > I am OK with adding these printings though (in addition to line numbers).
> >
> > I understand why you didn't like to see the file location, but what's the
> > problem with the line number?
> 
> +1 FWIW.  I found these per-line locations really useful when doing
> the strided load/store stuff.

Really?  Because the dumper always prints the location of the loop (i.e. 
its first line), not the location of the individual statements.  Therefore 
anything searching for file:line number will match all lines connected 
with dealing with one loop, there's no differentiation between them.  And 
hence I also don't see Iras point.  Searching for file:line from 
file start before Richis changes would get you to the start where the loop 
is dealt with, and then not a bit further because all lines would be so 
prefixed.   Searching from file end would get you to the end of the loop 
processing (with the final decision), and also not further because of the 
same prefix everywhere.

So, no, I don't see how to prefix every line with the same prefix provides 
anything useful.  Can you show me how it was useful to you?  (Or you Ira?)

FWIW my proposal would have been to do this:
file:123 starting loop so-and-so
... unprefixed lines all dealing with this loop
file:123 finished loop so-and-so


Ciao,
Michael.

Re: [PATCH] Make vectorizer dumps more comparable

[Richard Sandiford]

Michael Matz <matz@suse.de> writes:
>> Ira Rosen <IRAR@il.ibm.com> writes:
>> >> How about, as Micha suggested, print the location of the loop
>> >> we currently investigate from vectorize_loops () where we
>> >> call find_loop_location () instead?
>> >
>> > The problem is that a dump of a single loop can be pretty long, and "start
>> > to analyze loop..."/"finish to analyze loop..." may be not visible enough.
>> > I am OK with adding these printings though (in addition to line numbers).
>> >
>> > I understand why you didn't like to see the file location, but what's the
>> > problem with the line number?
>> 
>> +1 FWIW.  I found these per-line locations really useful when doing
>> the strided load/store stuff.
>
> Really?  Because the dumper always prints the location of the loop (i.e. 
> its first line), not the location of the individual statements.  Therefore 
> anything searching for file:line number will match all lines connected 
> with dealing with one loop, there's no differentiation between them.  And 
> hence I also don't see Iras point.  Searching for file:line from 
> file start before Richis changes would get you to the start where the loop 
> is dealt with, and then not a bit further because all lines would be so 
> prefixed.   Searching from file end would get you to the end of the loop 
> processing (with the final decision), and also not further because of the 
> same prefix everywhere.
>
> So, no, I don't see how to prefix every line with the same prefix provides 
> anything useful.  Can you show me how it was useful to you?

I suppose it's just personal preference.  I look at the dumps using less,
and search for the line number.  That highlights all the lines in the
loop I care about.  It just seemed like a very nice visual cue when
the relevant part of the dump was longer than a page.

Richard