diff mbox

Make vectorizer dumps more comparable

Message ID OF81FD7184.88F3B5C4-ONC22578FE.0028D474-C22578FE.0029508B@il.ibm.com
State New
Headers show

Commit Message

Ira Rosen Sept. 1, 2011, 7:31 a.m. UTC
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?


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));
>

Comments

Richard Biener Sept. 1, 2011, 7:33 a.m. UTC | #1
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));
> >
> 
>
Ira Rosen Sept. 1, 2011, 7:51 a.m. UTC | #2
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
Richard Sandiford Sept. 1, 2011, 8:06 a.m. UTC | #3
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
Richard Biener Sept. 1, 2011, 8:13 a.m. UTC | #4
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
> 
>
Ira Rosen Sept. 1, 2011, 9:22 a.m. UTC | #5
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
Richard Biener Sept. 1, 2011, 9:26 a.m. UTC | #6
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
> 
>
Michael Matz Sept. 1, 2011, 1:41 p.m. UTC | #7
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.
Richard Sandiford Sept. 1, 2011, 2:40 p.m. UTC | #8
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
diff mbox

Patch

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;
 }