Re: [PATCH] Convert more passes to new dump framework

2013-08-30 Thread Sharad Singhai
On Fri, Aug 30, 2013 at 9:58 PM, Teresa Johnson  wrote:
> Besides, we might also want to
> use the same machinery (dump_printf_loc etc) for dump file dumping.
> The current behavior of using '-details' to turn on opt-info-all
> messages for dump files are not desirable.

 Interestingly, this doesn't even work. When I do
 -fdump-ipa-inline-details=stderr (with my patch containing the inliner
 messages) I am not getting those inliner messages emitted to stderr.
 Even though in dumpfile.c "details" is set to (TDF_DETAILS |
 MSG_OPTIMIZED_LOCATIONS | MSG_MISSED_OPTIMIZATION | MSG_NOTE). I'm not
 sure why, but will need to debug this.
>>>
>>> It works for vectorizer pass.
>>
>> Ok, let me see what is going on - I just confirmed that it is not
>> working for the loop unroller messages either.
>>
>
> Found the issue. The stream was incorrectly being closed when it was
> stderr/stdout. So only the dump output before the first dump_finish
> call was being emitted to stderr. I fixed this the same way the
> alt_dump_file was being handled just below - don't close if it is
> stderr/stdout. Confirmed that this fixes the problem.
>
> (So the real ratio between the volume of -fdump-...=stderr and
> -fopt-info is much higher than what I reported in an earlier email)
>
> Is the following patch ok, pending regression tests?
>
> 2013-08-30  Teresa Johnson  
>
> * dumpfile.c (dump_finish): Don't close stderr/stdout.
>
> Index: dumpfile.c
> ===
> --- dumpfile.c  (revision 202059)
> +++ dumpfile.c  (working copy)
> @@ -450,7 +450,8 @@ dump_finish (int phase)
>if (phase < 0)
>  return;
>dfi = get_dump_file_info (phase);
> -  if (dfi->pstream)
> +  if (dfi->pstream && strcmp("stderr", dfi->pfilename) != 0
> +  && strcmp("stdout", dfi->pfilename) != 0)
>  fclose (dfi->pstream);
>
>if (dfi->alt_stream && strcmp("stderr", dfi->alt_filename) != 0

Yes, this is clearly a bug which I missed. Thanks for fixing it. Is it
feasible to add a test case for it?

Thanks,
Sharad

> Thanks,
> Teresa


Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-30 Thread Sharad Singhai
Resend to gcc-patches

I have addressed the comments by fixing all the minor issues,
bootstrapped and tested on x86_64. I did the recommended reshuffling
by moving non-tree code from tree-dump.c into a new file dumpfile.c.

I committed two successive revisions
r191883 Main patch with the dump infrastructure changes. However, I
accidentally left out a new file, dumpfile.c.
r191884 Added dumpfile.c, and did the renaming of dump_* functions
from gimple_pretty_print.[ch].

As things stand right now, r191883 is broken because of the missing
file 'dumpfile.c', which the very next commit fixes. Anyone who got
broken revision r191883, please svn update. I am really very sorry
about that.

I have a couple more minor patches which deal with renaming; I plan to
address those later.

Thanks,
Sharad

> On Thu, Sep 27, 2012 at 9:10 AM, Xinliang David Li 
> wrote:
>>
>> On Thu, Sep 27, 2012 at 4:35 AM, Sharad Singhai 
>> wrote:
>> > Thanks for the review. A couple of comments inline:
>> >
>> >> Some minor issues:
>> >>
>> >> * c/c-decl.c (c_write_global_declarations): Use different
>> >> method to
>> >> determine if the dump has ben initialized.
>> >> * cp/decl2.c (cp_write_global_declarations): Ditto.
>> >> * testsuite/gcc.target/i386/vect-double-1.c: Fix test.
>> >>
>> >> these subdirs all have their separate ChangeLog entry from where the
>> >> directory name is omitted.
>> >>
>> >> Index: tree-dump.c
>> >> ===
>> >> --- tree-dump.c (revision 191490)
>> >> +++ tree-dump.c (working copy)
>> >> @@ -24,9 +24,11 @@ along with GCC; see the file COPYING3.  If not see
>> >>  #include "coretypes.h"
>> >>  #include "tm.h"
>> >>  #include "tree.h"
>> >> +#include "gimple-pretty-print.h"
>> >>  #include "splay-tree.h"
>> >>  #include "filenames.h"
>> >>  #include "diagnostic-core.h"
>> >> +#include "rtl.h"
>> >>
>> >> what do you need gimple-pretty-print.h and rtl.h for?
>> >>
>> >> +
>> >> +extern void dump_bb (FILE *, basic_block, int, int);
>> >> +
>> >>
>> >> that should be declared in some header
>> >>
>> >> +/* Dump gimple statement GS with SPC indentation spaces and
>> >> +   EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is enabled.  */
>> >> +
>> >> +void
>> >> +dump_gimple_stmt (int dump_kind, int extra_dump_flags, gimple gs, int
>> >> spc)
>> >> +{
>> >>
>> >> the gimple stuff really belongs in to gimple-pretty-print.c
>> >
>> > This dump_gimple_stmt () is just a dispatcher, which uses internal
>> > data structure such as dump streams/flags. If I move it into
>> > gimple-pretty-print.c, then I would have to export those
>> > streams/flags. I was hoping to avoid it by keeping all dump_* ()
>> > methods together in dumpfile.c (earlier in tree-dump.c). Thus, later
>> > one could just make dump_file/dump_flags static when all the passes
>> > have converted to this scheme.
>> >
>>
>> You can make the flags/streams global but only expose them via inline
>> accessors in the header file.
>>
>> David
>>
>> >>
>> >> (parts of tree-dump.c should be moved to a new file dumpfile.c)
>> >>
>> >> +/* Dump tree T using EXTRA_DUMP_FLAGS on dump streams if DUMP_KIND is
>> >> +   enabled.  */
>> >> +
>> >> +void
>> >> +dump_generic_expr (int dump_kind, int extra_dump_flags, tree t)
>> >> +{
>> >>
>> >> belongs to tree-pretty-print.c (to where the routines are it calls)
>> >
>> > This is again a dispatcher for dump_generic_expr () which writes to
>> > the appropriate stream depending upon dump_kind.
>> >
>> >>
>> >> +int
>> >> +dump_start (int phase, int *flag_ptr)
>> >> +{
>> >>
>> >> perfect candidate for dumpfile.c
>> >>
>> >> You can do this re-shuffling as followup, but please try to not include
>> >> rtl.h
>> >> or gimple-pretty-print.h from tree-dump.c.  Thus re-shuffling required
>> >> by that
>> >> do now.  tree-dump.c should only know about dumping 'tree'.
>

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai
I am sorry, I didn't enable all the languages. Will fix the fortran
test breakage shortly.

Thanks,
Sharad
Sharad


On Mon, Oct 1, 2012 at 4:50 AM, H.J. Lu  wrote:
> On Sun, Sep 30, 2012 at 11:36 PM, Sharad Singhai  wrote:
>> Resend to gcc-patches
>>
>> I have addressed the comments by fixing all the minor issues,
>> bootstrapped and tested on x86_64. I did the recommended reshuffling
>> by moving non-tree code from tree-dump.c into a new file dumpfile.c.
>>
>> I committed two successive revisions
>> r191883 Main patch with the dump infrastructure changes. However, I
>> accidentally left out a new file, dumpfile.c.
>> r191884 Added dumpfile.c, and did the renaming of dump_* functions
>> from gimple_pretty_print.[ch].
>>
>> As things stand right now, r191883 is broken because of the missing
>> file 'dumpfile.c', which the very next commit fixes. Anyone who got
>> broken revision r191883, please svn update. I am really very sorry
>> about that.
>>
>> I have a couple more minor patches which deal with renaming; I plan to
>> address those later.
>>
>
> It caused:
>
> FAIL: gcc.dg/tree-ssa/gen-vect-11.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-11.c scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-11a.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-11a.c scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-11b.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-11b.c scan-tree-dump-times vect
> "vectorized 0 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-11c.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-11c.c scan-tree-dump-times vect
> "vectorized 0 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-2.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-2.c scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-25.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-25.c scan-tree-dump-times vect
> "vectorized 2 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-26.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-26.c scan-tree-dump-times vect
> "Alignment of access forced using peeling" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-26.c scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-28.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-28.c scan-tree-dump-times vect
> "Alignment of access forced using peeling" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-28.c scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gcc.dg/tree-ssa/gen-vect-32.c (test for excess errors)
> FAIL: gcc.dg/tree-ssa/gen-vect-32.c scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gfortran.dg/vect/O3-pr36119.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/O3-pr39595.f (test for excess errors)
> FAIL: gfortran.dg/vect/Ofast-pr50414.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/cost-model-pr34445.f (test for excess errors)
> FAIL: gfortran.dg/vect/cost-model-pr34445a.f (test for excess errors)
> FAIL: gfortran.dg/vect/fast-math-pr38968.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/fast-math-pr38968.f90 scan-tree-dump vect
> "vectorized 1 loops"
> FAIL: gfortran.dg/vect/fast-math-real8-pr40801.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/fast-math-real8-pr40801.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/fast-math-vect-8.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/fast-math-vect-8.f90 scan-tree-dump-times vect
> "vectorized 1 loops" 1
> FAIL: gfortran.dg/vect/no-fre-no-copy-prop-O3-pr51704.f90 (test for
> excess errors)
> FAIL: gfortran.dg/vect/no-vfa-pr32377.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/no-vfa-pr32377.f90 scan-tree-dump-times vect
> "vectorized 2 loops" 1
> FAIL: gfortran.dg/vect/no-vfa-pr32457.f90 (test for excess errors)
> FAIL: gfortran.dg/vect/no-vfa-pr32457.f90 scan-tree-dump-times vect
> "vectorized 0 loops" 1
> FAIL: gfortran.dg/vect/pr19049.f90  -O   scan-tree-dump-times vect
> "complicated access pattern" 1
> FAIL: gfortran.dg/vect/pr19049.f90  -O  (test for excess errors)
> FAIL: gfortran.dg/vect/pr32377.f90  -O   scan-tree-dump-times vect
> "vectorized 2 loops" 1
> FAIL: gfortran.dg/vect/pr32377.f90  -O  (test for excess errors)
> FAIL: gfortran.dg/vect/pr32380.f  -O   scan-tree-dump-times vect
> "vectorized 6 loops" 1
> FAIL: gfortran.dg/vect/pr32380.f  -O  (test for excess errors)
> FAIL: gfortran.dg/vect/pr33301.f  -O  (test for excess errors)
> FAIL: gfortra

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai
On Mon, Oct 1, 2012 at 6:52 AM, H.J. Lu  wrote:
> On Mon, Oct 1, 2012 at 6:49 AM, Sharad Singhai  wrote:
>> I am sorry, I didn't enable all the languages. Will fix the fortran
>> test breakage shortly.
>
> It is not just Fortran.  There are some failures in C testcases.

I checked and those files looked like generator files for Fortran
tests and thus were not exercised in my configuration. I am really
sorry about that. I am fixing it.

UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11a.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11b.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11c.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-2.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-25.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-26.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-28.c
UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-32.c

Thanks,
Sharad

>
>> Thanks,
>> Sharad
>> Sharad
>>
>>
>> On Mon, Oct 1, 2012 at 4:50 AM, H.J. Lu  wrote:
>>> On Sun, Sep 30, 2012 at 11:36 PM, Sharad Singhai  wrote:
>>>> Resend to gcc-patches
>>>>
>>>> I have addressed the comments by fixing all the minor issues,
>>>> bootstrapped and tested on x86_64. I did the recommended reshuffling
>>>> by moving non-tree code from tree-dump.c into a new file dumpfile.c.
>>>>
>>>> I committed two successive revisions
>>>> r191883 Main patch with the dump infrastructure changes. However, I
>>>> accidentally left out a new file, dumpfile.c.
>>>> r191884 Added dumpfile.c, and did the renaming of dump_* functions
>>>> from gimple_pretty_print.[ch].
>>>>
>>>> As things stand right now, r191883 is broken because of the missing
>>>> file 'dumpfile.c', which the very next commit fixes. Anyone who got
>>>> broken revision r191883, please svn update. I am really very sorry
>>>> about that.
>>>>
>>>> I have a couple more minor patches which deal with renaming; I plan to
>>>> address those later.
>>>>
>>>
>>> It caused:
>>>
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11.c scan-tree-dump-times vect
>>> "vectorized 1 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11a.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11a.c scan-tree-dump-times vect
>>> "vectorized 1 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11b.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11b.c scan-tree-dump-times vect
>>> "vectorized 0 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11c.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-11c.c scan-tree-dump-times vect
>>> "vectorized 0 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-2.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-2.c scan-tree-dump-times vect
>>> "vectorized 1 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-25.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-25.c scan-tree-dump-times vect
>>> "vectorized 2 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-26.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-26.c scan-tree-dump-times vect
>>> "Alignment of access forced using peeling" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-26.c scan-tree-dump-times vect
>>> "vectorized 1 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-28.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-28.c scan-tree-dump-times vect
>>> "Alignment of access forced using peeling" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-28.c scan-tree-dump-times vect
>>> "vectorized 1 loops" 1
>>> FAIL: gcc.dg/tree-ssa/gen-vect-32.c (test for excess errors)
>>> FAIL: gcc.dg/tree-ssa/gen-vect-32.c scan-tree-dump-times vect
>>> "vectorized 1 loops" 1
>>> FAIL: gfortran.dg/vect/O3-pr36119.f90 (test for excess errors)
>>> FAIL: gfortran.dg/vect/O3-pr39595.f (test for excess errors)
>>> FAIL: gfortran.dg/vect/Ofast-pr50414.f90 (test for excess errors)
>>> FAIL: gfortran.dg/vect/cost-model-pr34445.f (test for excess errors)
>>> FAIL: gfortran.dg/vect/cost-model-pr34445a.f (test for excess errors)
>>> FAIL: gfortran.dg/vect/fast-math-pr38968.f90 (test for excess errors)
>>> FAIL: gfortran.dg/vect/fast-math-pr38968.f90 scan-tree-dump vect
>>> "vectorized 1 loops"
>>> FAIL: gfortran.dg/vect/fast-math-real8-pr40801.f90 (test for excess errors)
>>> 

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai
Okay, I am retesting without any special configs and with multilibs as
you suggested.

Thanks,
Sharad

On Mon, Oct 1, 2012 at 7:00 AM, Richard Guenther
 wrote:
> On Mon, Oct 1, 2012 at 3:55 PM, Sharad Singhai  wrote:
>> On Mon, Oct 1, 2012 at 6:52 AM, H.J. Lu  wrote:
>>> On Mon, Oct 1, 2012 at 6:49 AM, Sharad Singhai  wrote:
>>>> I am sorry, I didn't enable all the languages. Will fix the fortran
>>>> test breakage shortly.
>>>
>>> It is not just Fortran.  There are some failures in C testcases.
>>
>> I checked and those files looked like generator files for Fortran
>> tests and thus were not exercised in my configuration. I am really
>> sorry about that. I am fixing it.
>
> As I said, you should not enable/disable anything special but
> configure with all default languages enabled (no --enable-languages)
> and do toplevel make -k check, preferably also excercising
> multilibs with RUNTESTFLAGS="--target_board=unix/\{,-m32\}"
>
> Richard.
>
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11a.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11b.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-11c.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-2.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-25.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-26.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-28.c
>> UNSUPPORTED: gcc.dg/tree-ssa/gen-vect-32.c
>>
>> Thanks,
>> Sharad
>>
>>>
>>>> Thanks,
>>>> Sharad
>>>> Sharad
>>>>
>>>>
>>>> On Mon, Oct 1, 2012 at 4:50 AM, H.J. Lu  wrote:
>>>>> On Sun, Sep 30, 2012 at 11:36 PM, Sharad Singhai  
>>>>> wrote:
>>>>>> Resend to gcc-patches
>>>>>>
>>>>>> I have addressed the comments by fixing all the minor issues,
>>>>>> bootstrapped and tested on x86_64. I did the recommended reshuffling
>>>>>> by moving non-tree code from tree-dump.c into a new file dumpfile.c.
>>>>>>
>>>>>> I committed two successive revisions
>>>>>> r191883 Main patch with the dump infrastructure changes. However, I
>>>>>> accidentally left out a new file, dumpfile.c.
>>>>>> r191884 Added dumpfile.c, and did the renaming of dump_* functions
>>>>>> from gimple_pretty_print.[ch].
>>>>>>
>>>>>> As things stand right now, r191883 is broken because of the missing
>>>>>> file 'dumpfile.c', which the very next commit fixes. Anyone who got
>>>>>> broken revision r191883, please svn update. I am really very sorry
>>>>>> about that.
>>>>>>
>>>>>> I have a couple more minor patches which deal with renaming; I plan to
>>>>>> address those later.
>>>>>>
>>>>>
>>>>> It caused:
>>>>>
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11.c scan-tree-dump-times vect
>>>>> "vectorized 1 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11a.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11a.c scan-tree-dump-times vect
>>>>> "vectorized 1 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11b.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11b.c scan-tree-dump-times vect
>>>>> "vectorized 0 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11c.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-11c.c scan-tree-dump-times vect
>>>>> "vectorized 0 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-2.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-2.c scan-tree-dump-times vect
>>>>> "vectorized 1 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-25.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-25.c scan-tree-dump-times vect
>>>>> "vectorized 2 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-26.c (test for excess errors)
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-26.c scan-tree-dump-times vect
>>>>> "Alignment of access forced using peeling" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-26.c scan-tree-dump-times vect
>>>>> "vectorized 1 loops" 1
>>>>> FAIL: gcc.dg/tree-ssa/gen-vect-28.

Re: [PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai
Thanks for tracking down and fixing the powerpc port.

The "dump_kind_p ()" check is redundant but canonical form here. I
think blocks of dump code guarded by "if dump_kind_p (...)" might be
easier to read/maintain.

Sharad
Sharad


On Mon, Oct 1, 2012 at 3:45 PM, Xinliang David Li  wrote:
> On Mon, Oct 1, 2012 at 2:37 PM, Michael Meissner
>  wrote:
>> I tracked down some of the other code that previously used REPORT_DETAILS, 
>> and
>> MSG_NOTE is the new way to do the same thing.  This bootstraps and no
>> unexpected errors occur during make check.  Is it ok to install?
>>
>> 2012-10-01  Michael Meissner  
>>
>> * config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
>>     (rs6000_density_test): Rework to accomidate 09-30 change by Sharad
>> Singhai.
>>
>> * config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.
>>
>> Index: gcc/config/rs6000/rs6000.c
>> ===
>> --- gcc/config/rs6000/rs6000.c  (revision 191932)
>> +++ gcc/config/rs6000/rs6000.c  (working copy)
>> @@ -58,6 +58,7 @@
>>  #include "tm-constrs.h"
>>  #include "opts.h"
>>  #include "tree-vectorizer.h"
>> +#include "dumpfile.h"
>>  #if TARGET_XCOFF
>>  #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
>>  #endif
>> @@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
>>&& vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
>>  {
>>data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
>> -  if (vect_print_dump_info (REPORT_DETAILS))
>> -   fprintf (vect_dump,
>> -"density %d%%, cost %d exceeds threshold, penalizing "
>> -"loop body cost by %d%%", density_pct,
>> -vec_cost + not_vec_cost, DENSITY_PENALTY);
>> +  if (dump_kind_p (MSG_NOTE))
>
> Is this check needed? Seems redundant.
>
> David
>
>
>> +   dump_printf_loc (MSG_NOTE, vect_location,
>> +"density %d%%, cost %d exceeds threshold, 
>> penalizing "
>> +"loop body cost by %d%%", density_pct,
>> +vec_cost + not_vec_cost, DENSITY_PENALTY);
>>  }
>>  }
>>
>> Index: gcc/config/rs6000/t-rs6000
>> ===
>> --- gcc/config/rs6000/t-rs6000  (revision 191932)
>> +++ gcc/config/rs6000/t-rs6000  (working copy)
>> @@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
>>$(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
>>output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
>>$(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
>> -  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
>> +  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h
>>
>>  rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
>>  $(srcdir)/config/rs6000/rs6000-protos.h \
>>
>> --
>> Michael Meissner, IBM
>> 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
>> meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
>>


Re: [PATCH] Fix test breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai
Here is a patch to fix test breakage caused by r191883. Bootstrapped
on x86_64 and tested with
make -k check RUNTESTFLAGS="--target_board=unix/\{,-m32\}".

Okay for trunk?

Thanks,
Sharad

2012-10-01  Sharad Singhai  

* tree-vect-stmts.c (vectorizable_operation): Add missing return.

testsuite/Changelog

* gfortran.dg/vect/vect.exp: Change verbose vectorizor dump options
to fix test failures caused by r191883.
* gcc.dg/tree-ssa/gen-vect-11.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-2.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-32.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-25.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-11a.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-26.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-11b.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-11c.c: Likewise.
* gcc.dg/tree-ssa/gen-vect-28.c: Likewise.
* testsuite/gcc.target/i386/vect-double-1.c: Fix test. Missing entry
from r191883.


Index: testsuite/gfortran.dg/vect/vect.exp
===
--- testsuite/gfortran.dg/vect/vect.exp (revision 191883)
+++ testsuite/gfortran.dg/vect/vect.exp (working copy)
@@ -26,7 +26,7 @@ set DEFAULT_VECTCFLAGS ""

 # These flags are used for all targets.
 lappend DEFAULT_VECTCFLAGS "-O2" "-ftree-vectorize" "-fno-vect-cost-model" \
-  "-ftree-vectorizer-verbose=4" "-fdump-tree-vect-stats"
+  "-fdump-tree-vect-details"

 # If the target system supports vector instructions, the default action
 # for a test is 'run', otherwise it's 'compile'.  Save current default.
Index: testsuite/gcc.dg/tree-ssa/gen-vect-11.c
===
--- testsuite/gcc.dg/tree-ssa/gen-vect-11.c (revision 191883)
+++ testsuite/gcc.dg/tree-ssa/gen-vect-11.c (working copy)
@@ -1,6 +1,6 @@
 /* { dg-do run { target vect_cmdline_needed } } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=3
-fwrapv -fdump-tree-vect-stats" } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=3
-fwrapv -fdump-tree-vect-stats -mno-sse" { target { i?86-*-*
x86_64-*-* } } } */
+/* { dg-options "-O2 -ftree-vectorize -fwrapv -fdump-tree-vect-details" } */
+/* { dg-options "-O2 -ftree-vectorize -fwrapv
-fdump-tree-vect-details -mno-sse" { target { i?86-*-* x86_64-*-* } }
} */

 #include 

Index: testsuite/gcc.dg/tree-ssa/gen-vect-2.c
===
--- testsuite/gcc.dg/tree-ssa/gen-vect-2.c (revision 191883)
+++ testsuite/gcc.dg/tree-ssa/gen-vect-2.c (working copy)
@@ -1,6 +1,6 @@
 /* { dg-do run { target vect_cmdline_needed } } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=4
-fdump-tree-vect-stats" } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=4
-fdump-tree-vect-stats -mno-sse" { target { i?86-*-* x86_64-*-* } } }
*/
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details" } */
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details
-mno-sse" { target { i?86-*-* x86_64-*-* } } } */

 #include 

Index: testsuite/gcc.dg/tree-ssa/gen-vect-32.c
===
--- testsuite/gcc.dg/tree-ssa/gen-vect-32.c (revision 191883)
+++ testsuite/gcc.dg/tree-ssa/gen-vect-32.c (working copy)
@@ -1,6 +1,6 @@
 /* { dg-do run { target vect_cmdline_needed } } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=4
-fdump-tree-vect-stats" } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=4
-fdump-tree-vect-stats -mno-sse" { target { i?86-*-* x86_64-*-* } } }
*/
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details" } */
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details
-mno-sse" { target { i?86-*-* x86_64-*-* } } } */

 #include 

Index: testsuite/gcc.dg/tree-ssa/gen-vect-25.c
===
--- testsuite/gcc.dg/tree-ssa/gen-vect-25.c (revision 191883)
+++ testsuite/gcc.dg/tree-ssa/gen-vect-25.c (working copy)
@@ -1,6 +1,6 @@
 /* { dg-do run { target vect_cmdline_needed } } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=4
-fdump-tree-vect-stats" } */
-/* { dg-options "-O2 -ftree-vectorize -ftree-vectorizer-verbose=4
-fdump-tree-vect-stats -mno-sse" { target { i?86-*-* x86_64-*-* } } }
*/
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details" } */
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-details
-mno-sse" { target { i?86-*-* x86_64-*-* } } } */

 #include 

Index: testsuite/gcc.dg/tree-ssa/gen-vect-11a.c
===
--- testsuite/g

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai
I have mailed a patch to fix test failures caused by r191884. Waiting
for an okay.

http://gcc.gnu.org/ml/gcc-patches/2012-10/msg00109.html

Thanks,
Sharad
Sharad


On Mon, Oct 1, 2012 at 11:39 AM, Gabriel Dos Reis
 wrote:
> On Mon, Oct 1, 2012 at 1:27 PM, Michael Meissner
>  wrote:
>> On Mon, Oct 01, 2012 at 02:02:26PM -0400, Michael Meissner wrote:
>>> Your change on September 30th, breaks the powerpc port because the
>>> REPORT_DETAILS value in the enumeration is no longer there, and the
>>> rs6000_density_test function was using that.  Please in the future, when you
>>> are making global changes, grep for uses of enum values in all of the 
>>> machine
>>> dependent directories so we can avoid breakage like this.
>>
>> Also, in looking at the changes, given we are already up to 28 TDF_ flags, I
>> would recommend immediately adding a new type that is the TDF flagword type.
>> Thus it will be a lot simpler when we add 4 more TDF flags and have to change
>> the type from int to HOST_WIDE_INT.
>
> Agreed that we need an abstraction here.
> -- Gaby


[PATCH] Fix dumps for IPA passes

2012-10-18 Thread Sharad Singhai
Hi,

This patch fixes a problem with the new dump infrastructure as discussed in
http://gcc.gnu.org/ml/gcc/2012-10/msg00227.html.

It removes a check for current_function_decl so that dumps will work
for IPA passes. In addition, this patch also adds a new inline
function to check if any dump files are available.

I have bootstrapped and tested on x86_64. Okay for trunk?

Thanks,
Sharad


2012-10-18  Sharad Singhai  

* dumpfile.c (dump_enabled_phase): New function.
(dump_enabled_p): Rename to dump_enabled_phase. Update all callers.
A new function with the same name to check if any of the dump files
is available.
(dump_kind_p): Remove check for current_function_decl. Add check for
dumpfile and alt_dump_file.
* dumpfile.h: Add declaration of dump_enabled_p.

Index: dumpfile.c
===
--- dumpfile.c (revision 192549)
+++ dumpfile.c (working copy)
@@ -35,7 +35,7 @@ static int alt_flags;/* current op
 static FILE *alt_dump_file = NULL;

 static void dump_loc (int, FILE *, source_location);
-static int dump_enabled_p (int);
+static int dump_enabled_phase (int);
 static FILE *dump_open_alternate_stream (struct dump_file_info *);

 /* Table of tree dump switches. This must be consistent with the
@@ -380,7 +380,7 @@ dump_start (int phase, int *flag_ptr)
   char *name;
   struct dump_file_info *dfi;
   FILE *stream;
-  if (phase == TDI_none || !dump_enabled_p (phase))
+  if (phase == TDI_none || !dump_enabled_phase (phase))
 return 0;

   dfi = get_dump_file_info (phase);
@@ -461,7 +461,7 @@ dump_begin (int phase, int *flag_ptr)
   struct dump_file_info *dfi;
   FILE *stream;

-  if (phase == TDI_none || !dump_enabled_p (phase))
+  if (phase == TDI_none || !dump_enabled_phase (phase))
 return NULL;

   name = get_dump_file_name (phase);
@@ -493,8 +493,8 @@ dump_begin (int phase, int *flag_ptr)
If PHASE is TDI_tree_all, return nonzero if any dump is enabled for
any phase.  */

-int
-dump_enabled_p (int phase)
+static int
+dump_enabled_phase (int phase)
 {
   if (phase == TDI_tree_all)
 {
@@ -514,6 +514,14 @@ dump_begin (int phase, int *flag_ptr)
 }
 }

+/* Return true if any of the dumps are enabled, false otherwise. */
+
+inline bool
+dump_enabled_p (void)
+{
+  return (dump_file || alt_dump_file);
+}
+
 /* Returns nonzero if tree dump PHASE has been initialized.  */

 int
@@ -834,9 +842,8 @@ opt_info_switch_p (const char *arg)
 bool
 dump_kind_p (int msg_type)
 {
-  if (!current_function_decl)
-return 0;
-  return ((msg_type & pflags) || (msg_type & alt_flags));
+  return (dump_file && (msg_type & pflags))
+|| (alt_dump_file && (msg_type & alt_flags));
 }

 /* Print basic block on the dump streams.  */
Index: dumpfile.h
===
--- dumpfile.h (revision 192549)
+++ dumpfile.h (working copy)
@@ -121,6 +121,7 @@ extern int dump_switch_p (const char *);
 extern int opt_info_switch_p (const char *);
 extern const char *dump_flag_name (int);
 extern bool dump_kind_p (int);
+extern inline bool dump_enabled_p (void);
 extern void dump_printf (int, const char *, ...) ATTRIBUTE_PRINTF_2;
 extern void dump_printf_loc (int, source_location,
  const char *, ...) ATTRIBUTE_PRINTF_3;


Re: [PATCH] Fix dumps for IPA passes

2012-10-19 Thread Sharad Singhai
As suggested in http://gcc.gnu.org/ml/gcc/2012-10/msg00285.html, I
have updated the attached patch to rename 'dump_enabled_phase' to
'dump_enabled_phase_p'. The 'dump_enabled_p ()' doesn't take any
argument and can be used as a predicate for the dump calls.

Once this patch gets in, the plan is to update the existing calls (in
vectorizer passes) of the form
  if (dump_kind_p (flags))
  dump_printf(flags, ...)

to

  if (dump_enabled_p ())
  dump_printf(flags, ...)

Bootstrapped and tested on x86_64 and didn't observe any new test
failures. Okay for trunk?

Thanks,
Sharad

2012-10-19  Sharad Singhai  

* dumpfile.c (dump_phase_enabled_p): Renamed dump_enabled_p. Update
all callers.
(dump_enabled_p): A new function to check if any of the dump files
is available.
(dump_kind_p): Remove check for current_function_decl. Add check for
dumpfile and alt_dump_file.
* dumpfile.h: Add declaration of dump_enabled_p.

Index: dumpfile.c
===
--- dumpfile.c (revision 192623)
+++ dumpfile.c (working copy)
@@ -35,7 +35,7 @@ static int alt_flags;/* current op
 static FILE *alt_dump_file = NULL;

 static void dump_loc (int, FILE *, source_location);
-static int dump_enabled_p (int);
+static int dump_phase_enabled_p (int);
 static FILE *dump_open_alternate_stream (struct dump_file_info *);

 /* Table of tree dump switches. This must be consistent with the
@@ -380,7 +380,7 @@ dump_start (int phase, int *flag_ptr)
   char *name;
   struct dump_file_info *dfi;
   FILE *stream;
-  if (phase == TDI_none || !dump_enabled_p (phase))
+  if (phase == TDI_none || !dump_phase_enabled_p (phase))
 return 0;

   dfi = get_dump_file_info (phase);
@@ -461,7 +461,7 @@ dump_begin (int phase, int *flag_ptr)
   struct dump_file_info *dfi;
   FILE *stream;

-  if (phase == TDI_none || !dump_enabled_p (phase))
+  if (phase == TDI_none || !dump_phase_enabled_p (phase))
 return NULL;

   name = get_dump_file_name (phase);
@@ -493,8 +493,8 @@ dump_begin (int phase, int *flag_ptr)
If PHASE is TDI_tree_all, return nonzero if any dump is enabled for
any phase.  */

-int
-dump_enabled_p (int phase)
+static int
+dump_phase_enabled_p (int phase)
 {
   if (phase == TDI_tree_all)
 {
@@ -514,6 +514,14 @@ dump_begin (int phase, int *flag_ptr)
 }
 }

+/* Return true if any of the dumps are enabled, false otherwise. */
+
+inline bool
+dump_enabled_p (void)
+{
+  return (dump_file || alt_dump_file);
+}
+
 /* Returns nonzero if tree dump PHASE has been initialized.  */

 int
@@ -834,9 +842,8 @@ opt_info_switch_p (const char *arg)
 bool
 dump_kind_p (int msg_type)
 {
-  if (!current_function_decl)
-return 0;
-  return ((msg_type & pflags) || (msg_type & alt_flags));
+  return (dump_file && (msg_type & pflags))
+|| (alt_dump_file && (msg_type & alt_flags));
 }

 /* Print basic block on the dump streams.  */
Index: dumpfile.h
===
--- dumpfile.h (revision 192623)
+++ dumpfile.h (working copy)
@@ -121,6 +121,7 @@ extern int dump_switch_p (const char *);
 extern int opt_info_switch_p (const char *);
 extern const char *dump_flag_name (int);
 extern bool dump_kind_p (int);
+extern inline bool dump_enabled_p (void);
 extern void dump_printf (int, const char *, ...) ATTRIBUTE_PRINTF_2;
 extern void dump_printf_loc (int, source_location,
  const char *, ...) ATTRIBUTE_PRINTF_3;


Re: [Patch] Fix the tests gcc.dg/vect/vect-8[23]_64.c

2012-10-23 Thread Sharad Singhai
+cc richard.guent...@gmail.com

If it is approved, I will be happy to commit it for you.

Thanks,
Sharad
Sharad


On Tue, Oct 23, 2012 at 6:52 AM, Dominique Dhumieres  wrote:
> Following the changes in [PATCH] Add option for dumping to stderr 
> (issue6190057)
> the tests gcc.dg/vect/vect-8[23]_64.c fails on powerpc*-*-*.
> This patch adjust the dump files and has been tested on powerpc-apple-darwin9.
>
> If approved could someone commit it for me (no write access).
>
> Note that these tests use both dg-do run and dg-do compile which is not
> supported (see http://gcc.gnu.org/ml/gcc/2012-10/msg00226.html and
> the rest of the thread).
>
> TIA
>
> Dominique
>
> gcc/testsuite/ChangeLog
> 2012-10-23  Dominique d'Humieres  
>
> * gcc.dg/vect/vect-82_64.c: Adjust the dump file.
> * gcc.dg/vect/vect-83_64.c: Likewise.
>
> diff -up gcc/testsuite/gcc.dg/vect/vect-82_64.c 
> ../work/gcc/testsuite/gcc.dg/vect/vect-82_64.c
> --- gcc/testsuite/gcc.dg/vect/vect-82_64.c  2007-11-21 20:18:48.0 
> +0100
> +++ ../work/gcc/testsuite/gcc.dg/vect/vect-82_64.c  2012-10-08 
> 13:52:25.0 +0200
> @@ -1,6 +1,6 @@
>  /* { dg-do run { target { { powerpc*-*-* && lp64 } && powerpc_altivec_ok } } 
> } */
>  /* { dg-do compile { target { { powerpc*-*-* && ilp32 } && 
> powerpc_altivec_ok } } } */
> -/* { dg-options "-O2 -ftree-vectorize -mpowerpc64 -fdump-tree-vect-stats 
> -maltivec" } */
> +/* { dg-options "-O2 -ftree-vectorize -mpowerpc64 -fdump-tree-vect-details 
> -maltivec" } */
>
>  #include 
>  #include "tree-vect.h"
> diff -up gcc/testsuite/gcc.dg/vect/vect-83_64.c 
> ../work/gcc/testsuite/gcc.dg/vect/vect-83_64.c
> --- gcc/testsuite/gcc.dg/vect/vect-83_64.c  2007-11-21 20:18:48.0 
> +0100
> +++ ../work/gcc/testsuite/gcc.dg/vect/vect-83_64.c  2012-10-08 
> 13:52:42.0 +0200
> @@ -1,6 +1,6 @@
>  /* { dg-do run { target { { powerpc*-*-* && lp64 } && powerpc_altivec_ok } } 
> } */
>  /* { dg-do compile { target { { powerpc*-*-* && ilp32 } && 
> powerpc_altivec_ok } } } */
> -/* { dg-options "-O2 -ftree-vectorize -mpowerpc64 -fdump-tree-vect-stats 
> -maltivec" } */
> +/* { dg-options "-O2 -ftree-vectorize -mpowerpc64 -fdump-tree-vect-details 
> -maltivec" } */
>
>  #include 
>  #include "tree-vect.h"


Re: PR tree-optimization/54985

2012-10-23 Thread Sharad Singhai
The trunk seems to be broken at r192749 due to this patch.

../../srctrunk/gcc/tree-ssa-threadedge.c: In function ‘void
thread_across_edge(gimple_statement_d*, edge_def*, bool,
vec_t**, tree_node* (*)(gimple_statement_d*,
gimple_statement_d*))’:
../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
basic_block_def*)’
../../srctrunk/gcc/tree-ssa-threadedge.c:746: error: at this point in file
../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
basic_block_def*)’
../../srctrunk/gcc/tree-ssa-threadedge.c:790: error: at this point in file
../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
basic_block_def*)’
../../srctrunk/gcc/tree-ssa-threadedge.c:849: error: at this point in file
make: *** [tree-ssa-threadedge.o] Error 1

Sharad


Re: PR tree-optimization/54985

2012-10-23 Thread Sharad Singhai
The following trivial patch seems to fix it.

Index: tree-ssa-threadedge.c
===
--- tree-ssa-threadedge.c (revision 192749)
+++ tree-ssa-threadedge.c (working copy)
@@ -743,7 +743,7 @@
  safe to thread this edge.  */
   if (e->flags & EDGE_DFS_BACK)
 {
-  if (cond_arg_set_in_bb (e, e->dest, 1))
+  if (cond_arg_set_in_bb (e, e->dest))
  goto fail;
 }

@@ -787,7 +787,7 @@
  of threading without having to re-run DOM or VRP.  */
   if (dest
   && ((e->flags & EDGE_DFS_BACK) == 0
-  || ! cond_arg_set_in_bb (taken_edge, e->dest, 2)))
+  || ! cond_arg_set_in_bb (taken_edge, e->dest)))
 {
   /* We don't want to thread back to a block we have already
  visited.  This may be overly conservative.  */
@@ -846,7 +846,7 @@
  do
   {
 if ((e->flags & EDGE_DFS_BACK) == 0
- || ! cond_arg_set_in_bb (e3, e->dest, 3))
+ || ! cond_arg_set_in_bb (e3, e->dest))
   e2 = thread_around_empty_block (e3,
   dummy_cond,
   handle_dominating_asserts,
Sharad


On Tue, Oct 23, 2012 at 4:48 PM, Sharad Singhai  wrote:
> The trunk seems to be broken at r192749 due to this patch.
>
> ../../srctrunk/gcc/tree-ssa-threadedge.c: In function ‘void
> thread_across_edge(gimple_statement_d*, edge_def*, bool,
> vec_t**, tree_node* (*)(gimple_statement_d*,
> gimple_statement_d*))’:
> ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
> arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
> basic_block_def*)’
> ../../srctrunk/gcc/tree-ssa-threadedge.c:746: error: at this point in file
> ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
> arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
> basic_block_def*)’
> ../../srctrunk/gcc/tree-ssa-threadedge.c:790: error: at this point in file
> ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
> arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
> basic_block_def*)’
> ../../srctrunk/gcc/tree-ssa-threadedge.c:849: error: at this point in file
> make: *** [tree-ssa-threadedge.o] Error 1
>
> Sharad


Re: [Patch] Fix the tests gcc.dg/vect/vect-8[23]_64.c

2012-10-23 Thread Sharad Singhai
Committed in r192750.

Thanks,
Sharad

On Tue, Oct 23, 2012 at 2:46 PM, Mike Stump  wrote:
> On Oct 23, 2012, at 6:52 AM, Dominique Dhumieres  wrote:
>> Following the changes in [PATCH] Add option for dumping to stderr 
>> (issue6190057)
>> the tests gcc.dg/vect/vect-8[23]_64.c fails on powerpc*-*-*.
>> This patch adjust the dump files and has been tested on 
>> powerpc-apple-darwin9.
>
> Ok.
>
>> gcc/testsuite/ChangeLog
>> 2012-10-23  Dominique d'Humieres  
>>
>>   * gcc.dg/vect/vect-82_64.c: Adjust the dump file.
>>   * gcc.dg/vect/vect-83_64.c: Likewise.
>


Add myself to MAINTAINERS

2012-10-24 Thread Sharad Singhai
Added myself as write after approval maintainer in r192781.

Thanks,
Sharad

Index: ChangeLog
===
--- ChangeLog (revision 192779)
+++ ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2012-10-24  Sharad Singhai  
+
+ * MAINTAINERS (Write After Approval): Add myself.
+
 2012-10-24  Eric Christopher  

  * MAINTAINERS: Update email address.
Index: MAINTAINERS
===
--- MAINTAINERS (revision 192779)
+++ MAINTAINERS (working copy)
@@ -502,6 +502,7 @@ Dodji Seketeli do...@gcc.gnu.org
 Svein Seldal sv...@dev.seldal.com
 Thiemo Seufer t...@networkno.de
 Marcus Shawcroft marcus.shawcr...@arm.com
+Sharad Singhai sing...@google.com
 Johannes Singler sing...@kit.edu
 Franz Sirl franz.sirl-ker...@lauterbach.com
 Jan Sjodin jan.sjo...@amd.com


[PATCH] obvious fix for rs6000 broken bootstrap committed

2012-10-24 Thread Sharad Singhai
As per discussion in http://gcc.gnu.org/ml/gcc/2012-10/msg00366.html.
I have applied the following obvious fix for rs6000 broken bootstrap.

2012-10-24  Sharad Singhai  

* config/rs6000/rs6000.c (rs6000_density_test): Use dump_enabled_p
  instead of dump_kind_p.

Index: config/rs6000/rs6000.c
===
--- config/rs6000/rs6000.c (revision 192787)
+++ config/rs6000/rs6000.c (working copy)
@@ -3547,7 +3547,7 @@ rs6000_density_test (rs6000_cost_data *data)
   && vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
 {
   data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
-  if (dump_kind_p (MSG_NOTE))
+  if (dump_enabled_p ())
  dump_printf_loc (MSG_NOTE, vect_location,
  "density %d%%, cost %d exceeds threshold, penalizing "
  "loop body cost by %d%%", density_pct,


Thanks,.
Sharad


Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon

2012-10-30 Thread Sharad Singhai
Hi Jakub,

My -fopt-info pass filtering patch
(http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02704.html) is being
reviewed and I hope to get this in by Nov. 5 for inclusion in gcc
4.8.0.

Thanks,
Sharad

On Mon, Oct 29, 2012 at 10:56 AM, Jakub Jelinek  wrote:
> Status
> ==
>
> I'd like to close the stage 1 phase of GCC 4.8 development
> on Monday, November 5th.  If you have still patches for new features you'd
> like to see in GCC 4.8, please post them for review soon.  Patches
> posted before the freeze, but reviewed shortly after the freeze, may
> still go in, further changes should be just bugfixes and documentation
> fixes.
>
>
> Quality Data
> 
>
> Priority  #   Change from Last Report
> ---   ---
> P1   23   + 23
> P2   77   +  8
> P3   85   + 84
> ---   ---
> Total   185   +115
>
>
> Previous Report
> ===
>
> http://gcc.gnu.org/ml/gcc/2012-03/msg00011.html
>
> The next report will be sent by me again, announcing end of stage 1.


[PATCH] Fix PR middle-end/58134

2013-10-29 Thread Sharad Singhai
This patch removes a deprecated option -ftree-vectorizer-verbose. It
was already implemented in terms of -fopt-info and makes little sense
to keep it around longer. It causes needless confusion as reported in
PR middle-end/58134.

I noticed that several gettext related gcc/po files contain help
translation for the option being removed. However, I haven't updated
them as I wasn't sure if these files should be regenerated using a
separate mechanism. If needed, I can include manual updates to those
.po files as well.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Okay
for the trunk?

Thanks,
Sharad

2013-10-29  Sharad Singhai  

PR middle-end/58134
* opts.c (common_handle_option): Remove deprecated option
-ftree-vectorizer-verbose.
* doc/invoke.texi (Debugging Options): Ditto.
* common.opt: Ditto.
* opts-global.c (handle_common_deferred_options): Ditto.
(dump_remap_tree_vectorizer_verbose): Delete.
2013-10-29  Sharad Singhai  

	PR middle-end/58134
	* opts.c (common_handle_option): Remove deprecated option
	-ftree-vectorizer-verbose.
	* doc/invoke.texi (Debugging Options): Ditto.
	* common.opt: Ditto.
	* opts-global.c (handle_common_deferred_options): Ditto.
	(dump_remap_tree_vectorizer_verbose): Delete.

Index: opts.c
===
--- opts.c	(revision 204038)
+++ opts.c	(working copy)
@@ -1789,12 +1789,6 @@ common_handle_option (struct gcc_options *opts,
   opts->x_flag_stack_usage_info = value != 0;
   break;
 
-case OPT_ftree_vectorizer_verbose_:
-  /* -ftree-vectorizer-verbose is deprecated. It is defined in
- -terms of fopt-info=N. */
-  /* Deferred.  */
-  break;
-
 case OPT_g:
   set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, arg, opts, opts_set,
 		   loc);
Index: doc/invoke.texi
===
--- doc/invoke.texi	(revision 204038)
+++ doc/invoke.texi	(working copy)
@@ -322,7 +322,6 @@ Objective-C and Objective-C++ Dialects}.
 -fdump-tree-fre@r{[}-@var{n}@r{]} @gol
 -fdump-tree-vtable-verify @gol
 -fdump-tree-vrp@r{[}-@var{n}@r{]} @gol
--ftree-vectorizer-verbose=@var{n} @gol
 -fdump-tree-storeccp@r{[}-@var{n}@r{]} @gol
 -fdump-final-insns=@var{file} @gol
 -fcompare-debug@r{[}=@var{opts}@r{]}  -fcompare-debug-second @gol
@@ -6376,24 +6375,6 @@ the first option takes effect and the subsequent o
 ignored. Thus only the @file{vec.miss} is produced which cotaints
 dumps from the vectorizer about missed opportunities.
 
-@item -ftree-vectorizer-verbose=@var{n}
-@opindex ftree-vectorizer-verbose
-This option is deprecated and is implemented in terms of
-@option{-fopt-info}. Please use @option{-fopt-info-@var{kind}} form
-instead, where @var{kind} is one of the valid opt-info options. It
-prints additional optimization information.  For @var{n}=0 no
-diagnostic information is reported.  If @var{n}=1 the vectorizer
-reports each loop that got vectorized, and the total number of loops
-that got vectorized.  If @var{n}=2 the vectorizer reports locations
-which could not be vectorized and the reasons for those. For any
-higher verbosity levels all the analysis and transformation
-information from the vectorizer is reported.
-
-Note that the information output by @option{-ftree-vectorizer-verbose}
-option is sent to @file{stderr}. If the equivalent form
-@option{-fopt-info-@var{options}=@var{filename}} is used then the
-output is sent into @var{filename} instead.
-
 @item -frandom-seed=@var{string}
 @opindex frandom-seed
 This option provides a seed that GCC uses in place of
Index: common.opt
===
--- common.opt	(revision 204038)
+++ common.opt	(working copy)
@@ -2270,10 +2270,6 @@ ftree-vectorize
 Common Report Var(flag_tree_vectorize) Optimization
 Enable vectorization on trees
 
-ftree-vectorizer-verbose=
-Common RejectNegative Joined UInteger Var(common_deferred_options) Defer
--ftree-vectorizer-verbose=	This switch is deprecated. Use -fopt-info instead.
-
 ftree-loop-vectorize
 Common Report Var(flag_tree_loop_vectorize) Optimization
 Enable loop vectorization on trees
Index: opts-global.c
===
--- opts-global.c	(revision 204038)
+++ opts-global.c	(working copy)
@@ -231,40 +231,6 @@ read_cmdline_options (struct gcc_options *opts, st
 }
 }
 
-/* Handle -ftree-vectorizer-verbose=ARG by remapping it to -fopt-info.
-   It remaps the old verbosity values as following:
-
-   REPORT_NONE ==> No dump is output
-   REPORT_VECTORIZED_LOCATIONS ==> "-optimized"
-   REPORT_UNVECTORIZED_LOCATIONS ==> "-missed"
-
-   Any higher verbosity levels get mapped to "-all" flags.  */
-
-static void
-dump_remap_tree_vectorizer_verbose (const char *arg)
-{
-  int value = atoi (arg);
-  const char *remapped_opt_info = NULL;

[PATCH] Documentation for dump and optinfo output

2013-12-19 Thread Sharad Singhai
I am really sorry, I forgot to add gcc internal documentation for dump
functions. Sorry about a very long delay.

The attached patch adds some documentation about how to use the new
dump infrastructure. This doc is attached to 'passes' documentation
node as that seemed the most logical place. Also, I am not sure what
level of details would be useful. I could perhaps add some more
details if needed.

I ran 'make doc' and browsed the info output for basic sanity. Okay for trunk?

Thanks,
Sharad
2013-12-19

* Makefile.in: Add optinfo.texi.
* doc/optinfo.texi: New documentation for optimization info.
* doc/passes.texi: Add new node.

Index: Makefile.in
===
--- Makefile.in (revision 206105)
+++ Makefile.in (working copy)
@@ -2807,7 +2807,7 @@ TEXI_GCCINT_FILES = gccint.texi gcc-common.texi gc
 configfiles.texi collect2.texi headerdirs.texi funding.texi\
 gnu.texi gpl_v3.texi fdl.texi contrib.texi languages.texi  \
 sourcebuild.texi gty.texi libgcc.texi cfg.texi tree-ssa.texi   \
-loop.texi generic.texi gimple.texi plugins.texi
+loop.texi generic.texi gimple.texi plugins.texi optinfo.texi
 
 TEXI_GCCINSTALL_FILES = install.texi install-old.texi fdl.texi \
 gcc-common.texi gcc-vers.texi
Index: doc/optinfo.texi
===
--- doc/optinfo.texi(revision 0)
+++ doc/optinfo.texi(revision 0)
@@ -0,0 +1,208 @@
+@c Copyright (C) 2013 Free Software Foundation, Inc.
+@c This is part of the GCC manual.
+@c For copying conditions, see the file gcc.texi.
+
+@cindex optimization dumps
+
+This section is describes dump infrastructure which is common to both
+pass dumps as well as optimization dumps. The goal for this
+infrastructure is to provide both gcc developers and users detailed
+information about various compiler transformations and optimizations.
+
+@menu
+* Dump setup:: Setup of optimization dumps.
+* Optimization groups::Groups made up of optimization passes.
+* Dump output verbosity::  How much information to dump.
+* Dump output files and streams::  Output file names and streams.
+* Dump types:: Various types of dump functions.
+* Dump examples::  Sample usage.
+@end menu
+
+@node Dump setup
+@subsection Dump setup
+@cindex dump setup
+
+A dump_manager class is defined in @file{dumpfile.h}. Various passes
+register dumping pass-specific information via @code{dump_register} in
+@file{passes.c}. During registration, an optimization pass can select
+its optimization group (@pxref{Optimization groups}). Then the
+optimization information from the entire group can be output via
+command-line switches. Note that if a pass does not fit into any of
+the pre-defined groups, it can select @code{OPTGROUP_NONE}.
+
+In general, a pass need not worry about its dump output file name,
+whether certain flags are enabled, etc. However, for legacy reasons,
+passes could also call @code{dump_begin} which returns a stream in
+case the particular pass has optimization dumps enabled. A pass could
+call @code{dump_end} when the dump has ended. These methods should go
+away once all the passes are converted to use the new dump
+infrastructure.
+
+The recommended way to setup the dump output is via @code{dump_start}
+and @code{dump_end}.
+
+@node Optimization groups
+@subsection Optimization groups
+@cindex optimization groups
+The optimization passes are grouped into several categories. Currently
+defined categories in @file{dumpfile.h} are
+
+@ftable @code
+
+@item OPTGROUP_IPA
+IPA optimization passes. Enabled by @option{-ipa}
+
+@item OPTGROUP_LOOP
+Loop optimization passes. Enabled by @option{-loop}.
+
+@item OPTGROUP_INLINE
+Inlining passes. Enabled by @option{-inline}.
+
+@item OPTGROUP_VEC
+Vectorization passes. Enabled by @option{-vec}.
+
+@item OPTGROUP_OTHER
+All other optimization passes which do not fall into one of the above.
+
+@item OPTGROUP_ALL
+All optimization passes. Enabled by @option{-all}.
+
+@end ftable
+
+By using groups a user could selectively enable optimization
+information only for a group of passes. By default, the optimization
+information for all the passes is dumped.
+
+@node Dump output files and streams
+@subsection Dump output files and streams
+@cindex optimization info file names
+
+There are two separate output streams available for outputting
+optimization information from passes. Note that both these streams
+accept @code{stderr} and @code{stdout} as valid streams and thus it is
+possible to dump output to standard output or error. This is specially
+handy for outputting all available information in a single file by
+redirecting @code{stderr}.
+
+@table @code
+@item @code{pstream}
+This stream is for pass-specific dump output. For example,
+@option{-fdump-tree-vect=foo.v} dumps tree ve

Re: [PATCH] Convert more passes to new dump framework

2013-12-20 Thread Sharad Singhai
Committed documentation as r206161. Sorry about the delay.

Thanks,
Sharad

On Thu, Nov 28, 2013 at 10:03 AM, Martin Jambor  wrote:
> Hi,
>
> On Tue, Aug 06, 2013 at 10:18:05AM -0700, Sharad Singhai wrote:
>> On Tue, Aug 6, 2013 at 10:10 AM, Martin Jambor  wrote:
>> > On Tue, Aug 06, 2013 at 09:22:02AM -0700, Sharad Singhai wrote:
>> >> On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  
>> >> wrote:
>> >> > On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>> >> >> Hi,
>> >> >>
>> >> >> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> >> >>> This patch ports messages to the new dump framework,
>> >> >>
>> >> >> It would be great this new framework was documented somewhere.  I lost
>> >> >> track of what was agreed it would be and from the uses in the
>> >> >> vectorizer I was never quite sure how to utilize it in other passes.
>> >> >
>> >> > Sharad, can you put the documentation in GCC wiki.
>> >>
>> >> Sure. I had user documentation in form of gcc info. But I will add
>> >> more developer details to a GCC wiki.
>> >>
>> >
>> > I have built trunk gccint.info yesterday but could not find any string
>> > dump_enabled_p there, for example.  And when I quickly searched just
>> > for the string "dump," I did not find any thing that looked like
>> > dumping infrastructure either.  OTOH, I agree that fie would be the
>> > best place for the documentation.
>> >
>> > Or did I just miss it?  What section is it in then?
>>
>> Actually, the user-facing documentation is in doc/invoke.texi.
>> However, that doesn't describe dump_enabled_p. Do you think
>> gccint.info would be a good place? I can add documentation there
>> instead of creating a GCC wiki.
>>
>
> please do not forget about this, otherwise few people will use your
> framework.
>
> Thanks,
>
> Martin


Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon

2012-11-01 Thread Sharad Singhai
On Tue, Oct 30, 2012 at 4:04 PM, Sharad Singhai  wrote:
> Hi Jakub,
>
> My -fopt-info pass filtering patch
> (http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02704.html) is being
> reviewed and I hope to get this in by Nov. 5 for inclusion in gcc
> 4.8.0.

I just committed -fopt-info pass filtering patch as r193061.

Thanks,
Sharad

> Thanks,
> Sharad
>
> On Mon, Oct 29, 2012 at 10:56 AM, Jakub Jelinek  wrote:
>> Status
>> ==
>>
>> I'd like to close the stage 1 phase of GCC 4.8 development
>> on Monday, November 5th.  If you have still patches for new features you'd
>> like to see in GCC 4.8, please post them for review soon.  Patches
>> posted before the freeze, but reviewed shortly after the freeze, may
>> still go in, further changes should be just bugfixes and documentation
>> fixes.
>>
>>
>> Quality Data
>> 
>>
>> Priority  #   Change from Last Report
>> ---   ---
>> P1   23   + 23
>> P2   77   +  8
>> P3   85   + 84
>> ---   ---
>> Total   185   +115
>>
>>
>> Previous Report
>> ===
>>
>> http://gcc.gnu.org/ml/gcc/2012-03/msg00011.html
>>
>> The next report will be sent by me again, announcing end of stage 1.


Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon

2012-11-01 Thread Sharad Singhai
I am really sorry about that. I am looking and will fix the breakage
or revert the patch shortly.

Thanks,
Sharad

On Thu, Nov 1, 2012 at 5:28 AM, Jakub Jelinek  wrote:
> On Thu, Nov 01, 2012 at 12:52:04AM -0700, Sharad Singhai wrote:
>> On Tue, Oct 30, 2012 at 4:04 PM, Sharad Singhai  wrote:
>> > Hi Jakub,
>> >
>> > My -fopt-info pass filtering patch
>> > (http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02704.html) is being
>> > reviewed and I hope to get this in by Nov. 5 for inclusion in gcc
>> > 4.8.0.
>>
>> I just committed -fopt-info pass filtering patch as r193061.
>
> How was that change tested?  I'm seeing thousands of new UNRESOLVED
> failures, of the form:
> spawn -ignore SIGHUP /usr/src/gcc/obj415/gcc/xgcc -B/usr/src/gcc/obj415/gcc/ 
> /usr/src/gcc/gcc/testsuite/gcc.target/i386/branch-cost1.c 
> -fno-diagnostics-show-caret -O2 -fdump-tree-gimple -mbranch-cost=0 -S -o 
> branch-cost1.s
> PASS: gcc.target/i386/branch-cost1.c (test for excess errors)
> gcc.target/i386/branch-cost1.c: dump file does not exist
> UNRESOLVED: gcc.target/i386/branch-cost1.c scan-tree-dump-times gimple "if " 2
> gcc.target/i386/branch-cost1.c: dump file does not exist
> UNRESOLVED: gcc.target/i386/branch-cost1.c scan-tree-dump-not gimple " & "
>
> See http://gcc.gnu.org/ml/gcc-testresults/2012-11/msg00033.html
> or http://gcc.gnu.org/ml/gcc-testresults/2012-11/msg00034.html, compare that
> to http://gcc.gnu.org/ml/gcc-testresults/2012-11/msg00025.html
> or http://gcc.gnu.org/ml/gcc-testresults/2012-11/msg00026.html
>
> The difference is just your patch and unrelated sh backend change.
>
> Jakub


Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon

2012-11-01 Thread Sharad Singhai
I found the problem and the following patch fixes it. The issue with
my testing was that I was only looking at 'FAIL' lines but forgot to
tally the 'UNRESOLVED' test cases, the real symptoms of my test
problems.  In any case,  I am rerunning the whole testsuite just to be
sure.

Assuming tests pass, is it okay to commit the following?

Thanks,
Sharad

2012-11-01  Sharad Singhai  

PR other/55164
* dumpfile.h (struct dump_file_info): Fix order of flags.

Index: dumpfile.h
===
--- dumpfile.h (revision 193061)
+++ dumpfile.h (working copy)
@@ -113,8 +113,8 @@ struct dump_file_info
   const char *alt_filename; /* filename for the -fopt-info stream  */
   FILE *pstream;/* pass-specific dump stream  */
   FILE *alt_stream; /* -fopt-info stream */
+  int pflags;   /* dump flags */
   int optgroup_flags;   /* optgroup flags for -fopt-info */
-  int pflags;   /* dump flags */
   int alt_flags;/* flags for opt-info */
   int pstate;   /* state of pass-specific stream */
   int alt_state;/* state of the -fopt-info stream */


Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon

2012-11-01 Thread Sharad Singhai
On Thu, Nov 1, 2012 at 9:44 AM, Diego Novillo  wrote:
> On Thu, Nov 1, 2012 at 12:40 PM, Sharad Singhai  wrote:
>> I found the problem and the following patch fixes it. The issue with
>> my testing was that I was only looking at 'FAIL' lines but forgot to
>> tally the 'UNRESOLVED' test cases, the real symptoms of my test
>> problems.  In any case,  I am rerunning the whole testsuite just to be
>> sure.
>>
>> Assuming tests pass, is it okay to commit the following?
>>
>> Thanks,
>> Sharad
>>
>> 2012-11-01  Sharad Singhai  
>>
>> PR other/55164
>> * dumpfile.h (struct dump_file_info): Fix order of flags.
>
> OK (remember to insert a tab at the start of each ChangeLog line).

Fixed tab chars. (they were really there, but gmail ate them! :))

Retested and found all my 'UNRESOLVED' problems were gone. Hence
committed the fix as r193064.

Thanks,
Sharad

>
> Diego.


[patch][google/gcc-4.8] Port gcov intermediate format from google/gcc-4.7

2013-05-24 Thread Sharad Singhai
Hi,

This patch forward ports r175134 from google/gcc-4.7 into
google/gcc-4.8. The intermediate format is a bit simplified. I am also
planning to propose this for trunk in a separate message.

Bootstrapped and tested on x86_64. Okay for google/gcc-4_8?

Thanks,
Sharad

2013-05-24   Sharad Singhai  

* gcov.c (print_usage): Handle new options.
(process_args): Handle new options.
(get_gcov_file_intermediate_name): New function.
(output_intermediate_file): New function.
(generate_results): Handle new option.
(release_function): Relase demangled name.
(read_graph_file): Handle demangled name.
(output_lines): Ditto.
* doc/gcov.texi: Document gcov intermediate format.

testsuite/ChangeLog:

2013-05-24   Sharad Singhai  

* g++.dg/gcov/gcov-8.C: New testcase.
* lib/gcov.exp: Handle intermediate format.


Index: testsuite/g++.dg/gcov/gcov-8.C
===
--- testsuite/g++.dg/gcov/gcov-8.C (revision 0)
+++ testsuite/g++.dg/gcov/gcov-8.C (revision 0)
@@ -0,0 +1,35 @@
+/* Verify that intermediate coverage format can be generated for
simple code. */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+class C {
+public:
+  C()
+  {
+i = 0;
+  }
+  ~C() {}
+  void seti (int j)
+  {
+if (j > 0)
+  i = j;
+else
+  i = 0;
+  }
+private:
+  int i;
+};
+
+void foo()
+{
+  C c;
+  c.seti (1);
+}
+
+int main()
+{
+  foo();
+}
+
+/* { dg-final { run-gcov intermediate { -i -b gcov-8.C } } } */
Index: testsuite/lib/gcov.exp
===
--- testsuite/lib/gcov.exp (revision 199269)
+++ testsuite/lib/gcov.exp (working copy)
@@ -70,7 +70,62 @@ proc verify-lines { testname testcase file } {
 return $failed
 }

+
 #
+# verify-intermediate -- check that intermediate file has certain lines
+#
+# TESTNAME is the name of the test, including unique flags.
+# TESTCASE is the name of the test.
+# FILE is the name of the gcov output file.
+#
+# Checks are very loose, they are based on certain tags being present
+# in the output. They do not check for exact expected execution
+# counts. For that the regular gcov format should be checked.
+#
+proc verify-intermediate { testname testcase file } {
+set failed 0
+set srcfile 0
+set function 0
+set lcount 0
+set branch 0
+set fd [open $file r]
+while { [gets $fd line] >= 0 } {
+ if [regexp "^file:" $line] {
+incr srcfile
+ }
+ if [regexp "^function:(\[0-9\]+),(\[0-9\]+),.*" $line] {
+incr function
+ }
+ if [regexp "^lcount:(\[0-9\]+),(\[0-9\]+)" $line] {
+incr lcount
+ }
+ if [regexp "^branch:(\[0-9\]+),(taken|nottaken|notexec)" $line] {
+incr branch
+ }
+}
+
+# We should see at least one tag of each type
+if {$srcfile == 0} {
+ fail "$testname expected 'file:' tag not found"
+ incr failed
+}
+if {$function == 0} {
+ fail "$testname expected 'function:' tag not found"
+ incr failed
+}
+if {$lcount == 0} {
+ fail "$testname expected 'lcount:' tag not found"
+ incr failed
+}
+if {$branch == 0} {
+ fail "$testname expected 'branch:' tag not found"
+ incr failed
+}
+return $failed
+}
+
+
+#
 # verify-branches -- check that branch percentages are as expected
 #
 # TESTNAME is the name of the test, including unique flags.
@@ -248,6 +303,8 @@ proc run-gcov { args } {
 set gcov_args ""
 set gcov_verify_calls 0
 set gcov_verify_branches 0
+set gcov_verify_lines 1
+set gcov_verify_intermediate 0
 set gcov_execute_xfail ""
 set gcov_verify_xfail ""
 set xfailed 0
@@ -257,6 +314,11 @@ proc run-gcov { args } {
   set gcov_verify_calls 1
  } elseif { $a == "branches" } {
   set gcov_verify_branches 1
+ } elseif { $a == "intermediate" } {
+  set gcov_verify_intermediate 1
+  set gcov_verify_calls 0
+  set gcov_verify_branches 0
+  set gcov_verify_lines 0
  } elseif { $gcov_args == "" } {
 set gcov_args $a
  } else {
@@ -297,7 +359,12 @@ proc run-gcov { args } {
 remote_upload host $testcase.gcov $testcase.gcov

 # Check that line execution counts are as expected.
-set lfailed [verify-lines $testname $testcase $testcase.gcov]
+if { $gcov_verify_lines } {
+ # Check that line execution counts are as expected.
+ set lfailed [verify-lines $testname $testcase $testcase.gcov]
+} else {
+ set lfailed 0
+}

 # If requested via the .x file, check that branch and call information
 # is correct.
@@ -311,15 +378,21 @@ proc run-gcov { args } {
 } else {
  set cfailed 0
 }
+if { $gcov_verify_intermediate } {
+ # Check that intermediate format has the expected format
+ set ifailed [verify-intermediate $testname $testcase $testcase.gcov]
+} else {
+ set ifailed 0
+}

 # Report w

[patch] gcov intermediate format

2013-05-28 Thread Sharad Singhai
This patch adds an intermediate coverage format ('gcov -i'). This is a
compact format as it does not require source files.

The new option ('gcov -i') outputs .gcov files in an intermediate text
format that can be post-processed by lcov or other tools.  It outputs a
single *.gcov file per *.gcda file. No source code is required.

The format of the intermediate .gcov file is plain text with one entry
per line. The output is tagged with one of the following

  file:source_file_name
  function:line_number,execution_count,function_name
  lcount:line number,execution_count
  branch:line_number,branch_coverage_type

Where the branch_coverage_type is
  notexec (Branch not executed)
  taken (Branch executed and taken)
  nottaken (Branch executed, but not taken)

There can be multiple 'file:' lines in an intermediate gcov file.  All
entries following 'file:' pertain to that source file until the next
line starting with 'file:'.

A sample 'gcov -i' output looks like this

  file:array.cc
  function:11,1,_Z3sumRKSt6vectorIPiSaIS0_EE
  function:22,1,main
  lcount:11,1
  lcount:12,8
  lcount:14,8
  branch:14,taken
  lcount:26,1
  branch:28,nottaken

By default the function names are mangled but using '-m' they can be
output as unmangled instead.

I have bootstrapped and tested this patch on x86_64-linux. No new test
failures were observed. Okay for trunk?

thanks,
Sharad

[This patch is already proposed for google branches in
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01546.html but here I
propose it for trunk.]

2013-05-28

* gcov.c (print_usage): Handle new option.
(process_args): Ditto.
(get_gcov_intermediate_filename): New function.
(output_intermediate_file): New function.
(output_gcov_file): New function
(generate_results): Handle new option.
(release_function): Relase demangled name.
(read_graph_file): Handle demangled name.
(output_lines): Ditto.
* doc/gcov.texi: Document gcov intermediate format.

testsuite/ChangeLog:

2013-05-28   Sharad Singhai  

* g++.dg/gcov/gcov-8.C: New testcase.
* lib/gcov.exp: Handle intermediate format.


Index: gcov.c
===
--- gcov.c (revision 199273)
+++ gcov.c (working copy)
@@ -37,6 +37,7 @@ along with Gcov; see the file COPYING3.  If not se
 #include "intl.h"
 #include "diagnostic.h"
 #include "version.h"
+#include "demangle.h"

 #include 

@@ -168,6 +169,7 @@ typedef struct function_info
 {
   /* Name of function.  */
   char *name;
+  char *demangled_name;
   unsigned ident;
   unsigned lineno_checksum;
   unsigned cfg_checksum;
@@ -325,6 +327,14 @@ static int flag_gcov_file = 1;

 static int flag_display_progress = 0;

+/* Output *.gcov file in intermediate format used by 'lcov'.  */
+
+static int flag_intermediate_format = 0;
+
+/* Output demangled function names.  */
+
+static int flag_demangled_names = 0;
+
 /* For included files, make the gcov output file name include the name
of the input source file.  For example, if x.h is included in a.c,
then the output file name is a.c##x.h.gcov instead of x.h.gcov.  */
@@ -388,6 +398,7 @@ static void executed_summary (unsigned, unsigned);
 static void function_summary (const coverage_t *, const char *);
 static const char *format_gcov (gcov_type, gcov_type, int);
 static void accumulate_line_counts (source_t *);
+static void output_gcov_file(const char *, source_t *);
 static int output_branch_count (FILE *, int, const arc_t *);
 static void output_lines (FILE *, const source_t *);
 static char *make_gcov_file_name (const char *, const char *);
@@ -461,21 +472,23 @@ print_usage (int error_p)
   fnotice (file, "Usage: gcov [OPTION]... SOURCE|OBJ...\n\n");
   fnotice (file, "Print code coverage information.\n\n");
   fnotice (file, "  -h, --help  Print this help,
then exit\n");
-  fnotice (file, "  -v, --version   Print version
number, then exit\n");
   fnotice (file, "  -a, --all-blocksShow information
for every basic block\n");
   fnotice (file, "  -b, --branch-probabilities  Include branch
probabilities in output\n");
-  fnotice (file, "  -c, --branch-counts Given counts of
branches taken\n\
+  fnotice (file, "  -c, --branch-counts Output counts of
branches taken\n\
 rather than percentages\n");
-  fnotice (file, "  -n, --no-output Do not create an
output file\n");
+  fnotice (file, "  -d, --display-progress  Display progress
information\n");
+  fnotice (file, "  -f, --function-summariesOutput summaries
for each function\n");
+  fnotice (file, "  -i, --intermediate-format   Output .gcov file
in intermediate text format\n");
   fnotice (file, "  -l, --long-file-names

Re: [patch] gcov intermediate format

2013-05-28 Thread Sharad Singhai
Sorry, my patch had bad formatting in one of the functions
(output_gcov_file). Here is the corrected version.

Thanks,
Sharad

(2013-05-28

* gcov.c (print_usage): Handle new option.
(process_args): Ditto.
(get_gcov_intermediate_filename): New function.
(output_intermediate_file): New function.
(output_gcov_file): New function
(generate_results): Handle new option.
(release_function): Relase demangled name.
(read_graph_file): Handle demangled name.
(output_lines): Ditto.
* doc/gcov.texi: Document gcov intermediate format.

testsuite/ChangeLog:

2013-05-28   Sharad Singhai  

* g++.dg/gcov/gcov-8.C: New testcase.
* lib/gcov.exp: Handle intermediate format.

Index: doc/gcov.texi
===
--- doc/gcov.texi (revision 199273)
+++ doc/gcov.texi (working copy)
@@ -122,15 +122,17 @@ gcov [@option{-v}|@option{--version}] [@option{-h}
  [@option{-a}|@option{--all-blocks}]
  [@option{-b}|@option{--branch-probabilities}]
  [@option{-c}|@option{--branch-counts}]
- [@option{-u}|@option{--unconditional-branches}]
+ [@option{-d}|@option{--display-progress}]
+ [@option{-f}|@option{--function-summaries}]
+ [@option{-i}|@option{--intermediate-format}]
+ [@option{-l}|@option{--long-file-names}]
+ [@option{-m}|@option{--demangled-names}]
  [@option{-n}|@option{--no-output}]
- [@option{-l}|@option{--long-file-names}]
+ [@option{-o}|@option{--object-directory} @var{directory|file}]
  [@option{-p}|@option{--preserve-paths}]
  [@option{-r}|@option{--relative-only}]
- [@option{-f}|@option{--function-summaries}]
- [@option{-o}|@option{--object-directory} @var{directory|file}]
  [@option{-s}|@option{--source-prefix} @var{directory}]
- [@option{-d}|@option{--display-progress}]
+ [@option{-u}|@option{--unconditional-branches}]
  @var{files}
 @c man end
 @c man begin SEEALSO
@@ -232,6 +234,50 @@ Unconditional branches are normally not interestin
 @itemx --display-progress
 Display the progress on the standard output.

+@item -i
+@itemx --intermediate-format
+Output gcov file in an easy-to-parse intermediate text format that can
+be used by @command{lcov} or other tools. The output is a single
+@file{.gcov} file per @file{.gcda} file. No source code is required.
+
+The format of the intermediate @file{.gcov} file is plain text with
+one entry per line
+
+@smallexample
+file:@var{source_file_name}
+function:@var{line_number},@var{execution_count},@var{function_name}
+lcount:@var{line number},@var{execution_count}
+branch:@var{line_number},@var{branch_coverage_type}
+
+Where the @var{branch_coverage_type} is
+   notexec (Branch not executed)
+   taken (Branch executed and taken)
+   nottaken (Branch executed, but not taken)
+
+There can be multiple @var{file} entries in an intermediate gcov
+file. All entries following a @var{file} pertain to that source file
+until the next @var{file} entry.
+@end smallexample
+
+Here is a sample when @option{-i} is used in conjuction with
@option{-b} option:
+
+@smallexample
+file:array.cc
+function:11,1,_Z3sumRKSt6vectorIPiSaIS0_EE
+function:22,1,main
+lcount:11,1
+lcount:12,1
+lcount:14,1
+branch:14,taken
+lcount:26,1
+branch:28,nottaken
+@end smallexample
+
+@item -m
+@itemx --demangled-names
+Display demangled function names in output. The default is to show
+mangled function names.
+
 @end table

 @command{gcov} should be run with the current directory the same as that
Index: gcov.c
===
--- gcov.c (revision 199273)
+++ gcov.c (working copy)
@@ -37,6 +37,7 @@ along with Gcov; see the file COPYING3.  If not se
 #include "intl.h"
 #include "diagnostic.h"
 #include "version.h"
+#include "demangle.h"

 #include 

@@ -168,6 +169,7 @@ typedef struct function_info
 {
   /* Name of function.  */
   char *name;
+  char *demangled_name;
   unsigned ident;
   unsigned lineno_checksum;
   unsigned cfg_checksum;
@@ -325,6 +327,14 @@ static int flag_gcov_file = 1;

 static int flag_display_progress = 0;

+/* Output *.gcov file in intermediate format used by 'lcov'.  */
+
+static int flag_intermediate_format = 0;
+
+/* Output demangled function names.  */
+
+static int flag_demangled_names = 0;
+
 /* For included files, make the gcov output file name include the name
of the input source file.  For example, if x.h is included in a.c,
then the output file name is a.c##x.h.gcov instead of x.h.gcov.  */
@@ -388,6 +398,7 @@ static void executed_summary (unsigned, unsigned);
 static void function_summary (const coverage_t *, const char *);
 static const char *format_gcov (gcov_type, gcov_type, int);
 static void accumulate_line_counts (source_t *);
+static void output_gcov_file(const char *, source_t *);
 static int output_branch_count (FILE *, int, const arc_t *);
 static void output_lines (FILE *, const source_t *);
 static char *make_gcov_file_nam

Re: [patch][google/gcc-4.8] Port gcov intermediate format from google/gcc-4.7

2013-05-28 Thread Sharad Singhai
On Fri, May 24, 2013 at 7:42 PM, Xinliang David Li  wrote:
> On Fri, May 24, 2013 at 2:32 PM, Sharad Singhai  wrote:
>>if (flag_gcov_file)
>>   {
>> -  char *gcov_file_name
>> -= make_gcov_file_name (file_name, src->coverage.name);
>> +  if (flag_intermediate_format)
>> +/* Output the intermediate format without requiring source
>> +   files.  This outputs a section to a *single* file.  */
>> +output_intermediate_file (gcov_file_intermediate, src);
>
> Is there an indentation issue here?

Yes, fixed.

>
>
>
>> +  else
>> +{
>> +  char *gcov_file_name
>> += make_gcov_file_name (file_name, src->coverage.name);
>>
>
>
> Try to avoid the following format only changes by reorganization --
> for instance refactor the following part into a helper function.

Fixed by adding helper function 'output_gcov_file'.

>
> Ok for google branch with those change.  Please submit the change to trunk 
> asap.

Proposed for trunk in
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01657.html. I will commit
this patch to google 4.8 branch after one more round of testing.

Thanks,
Sharad

>
> thanks,
>
> David
>
>> -  if (src->coverage.lines)
>> -{
>> -  FILE *gcov_file = fopen (gcov_file_name, "w");
>> +  if (src->coverage.lines)
>> +{
>> +  FILE *gcov_file = fopen (gcov_file_name, "w");
>>
>> -  if (gcov_file)
>> - {
>> -  fnotice (stdout, "Creating '%s'\n", gcov_file_name);
>> -  output_lines (gcov_file, src);
>> -  if (ferror (gcov_file))
>> -fnotice (stderr, "Error writing output file '%s'\n",
>> - gcov_file_name);
>> -  fclose (gcov_file);
>> - }
>> -  else
>> - fnotice (stderr, "Could not open output file '%s'\n",
>> - gcov_file_name);
>> -}
>> -  else
>> -{
>> -  unlink (gcov_file_name);
>> -  fnotice (stdout, "Removing '%s'\n", gcov_file_name);
>> -}
>> -  free (gcov_file_name);
>> - }
>> -  fnotice (stdout, "\n");
>> +  if (gcov_file)
>> +{
>> +  fnotice (stdout, "Creating '%s'\n", gcov_file_name);
>> +  output_lines (gcov_file, src);
>> +  if (ferror (gcov_file))
>> +fnotice (stderr, "Error writing output file '%s'\n",
>> + gcov_file_name);
>> +  fclose (gcov_file);
>> +}
>> +  else
>> +fnotice (stderr, "Could not open output file '%s'\n",
>> + gcov_file_name);
>> +}
>> +  else
>> +{
>> +  unlink (gcov_file_name);
>> +  fnotice (stdout, "Removing '%s'\n", gcov_file_name);
>> +}
>> +  free (gcov_file_name);
>> +}
>> +  fnotice (stdout, "\n");
>> +}
>>  }
>>
>> +  if (flag_gcov_file && flag_intermediate_format)
>> +{
>> +  /* Now we've finished writing the intermediate file.  */
>> +  fclose (gcov_file_intermediate);
>> +  XDELETEVEC (gcov_file_intermediate_name);
>> +}
>> +
>>if (!file_name)
>>  executed_summary (total_lines, total_executed);
>>  }
>> @@ -766,6 +914,9 @@ release_function (function_t *fn)
>>  }
>>free (fn->blocks);
>>free (fn->counts);
>> +  if (flag_demangled_names && fn->demangled_name != fn->name)
>> +free (fn->demangled_name);
>> +  free (fn->name);
>>  }
>>
>>  /* Release all memory used.  */
>> @@ -1051,6 +1202,12 @@ read_graph_file (void)
>>
>>fn = XCNEW (function_t);
>>fn->name = function_name;
>> +  if (flag_demangled_names)
>> +{
>> +  fn->demangled_name = cplus_demangle (fn->name, DMGL_PARAMS);
>> +  if (!fn->demangled_name)
>> +fn->demangled_name = fn->name;
>> +}
>>fn->ident = ident;
>>fn->lineno_checksum = lineno_checksum;
>>fn->cfg_checksum = cfg_checksum;
>> @@

Re: [patch][google/gcc-4.8] Port gcov intermediate format from google/gcc-4.7

2013-05-28 Thread Sharad Singhai
All tests passed. Committed as r199390 in google/gcc-4_8.

Thanks,
Sharad

On Tue, May 28, 2013 at 11:45 AM, Sharad Singhai  wrote:
> On Fri, May 24, 2013 at 7:42 PM, Xinliang David Li  wrote:
>> On Fri, May 24, 2013 at 2:32 PM, Sharad Singhai  wrote:
>>>if (flag_gcov_file)
>>>   {
>>> -  char *gcov_file_name
>>> -= make_gcov_file_name (file_name, src->coverage.name);
>>> +  if (flag_intermediate_format)
>>> +/* Output the intermediate format without requiring source
>>> +   files.  This outputs a section to a *single* file.  */
>>> +output_intermediate_file (gcov_file_intermediate, src);
>>
>> Is there an indentation issue here?
>
> Yes, fixed.
>
>>
>>
>>
>>> +  else
>>> +{
>>> +  char *gcov_file_name
>>> += make_gcov_file_name (file_name, src->coverage.name);
>>>
>>
>>
>> Try to avoid the following format only changes by reorganization --
>> for instance refactor the following part into a helper function.
>
> Fixed by adding helper function 'output_gcov_file'.
>
>>
>> Ok for google branch with those change.  Please submit the change to trunk 
>> asap.
>
> Proposed for trunk in
> http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01657.html. I will commit
> this patch to google 4.8 branch after one more round of testing.
>
> Thanks,
> Sharad
>
>>
>> thanks,
>>
>> David
>>
>>> -  if (src->coverage.lines)
>>> -{
>>> -  FILE *gcov_file = fopen (gcov_file_name, "w");
>>> +  if (src->coverage.lines)
>>> +{
>>> +  FILE *gcov_file = fopen (gcov_file_name, "w");
>>>
>>> -  if (gcov_file)
>>> - {
>>> -  fnotice (stdout, "Creating '%s'\n", gcov_file_name);
>>> -  output_lines (gcov_file, src);
>>> -  if (ferror (gcov_file))
>>> -fnotice (stderr, "Error writing output file '%s'\n",
>>> - gcov_file_name);
>>> -  fclose (gcov_file);
>>> - }
>>> -  else
>>> - fnotice (stderr, "Could not open output file '%s'\n",
>>> - gcov_file_name);
>>> -}
>>> -  else
>>> -{
>>> -  unlink (gcov_file_name);
>>> -  fnotice (stdout, "Removing '%s'\n", gcov_file_name);
>>> -}
>>> -  free (gcov_file_name);
>>> - }
>>> -  fnotice (stdout, "\n");
>>> +  if (gcov_file)
>>> +{
>>> +  fnotice (stdout, "Creating '%s'\n", gcov_file_name);
>>> +  output_lines (gcov_file, src);
>>> +  if (ferror (gcov_file))
>>> +fnotice (stderr, "Error writing output file 
>>> '%s'\n",
>>> + gcov_file_name);
>>> +  fclose (gcov_file);
>>> +}
>>> +  else
>>> +fnotice (stderr, "Could not open output file '%s'\n",
>>> + gcov_file_name);
>>> +}
>>> +  else
>>> +{
>>> +  unlink (gcov_file_name);
>>> +  fnotice (stdout, "Removing '%s'\n", gcov_file_name);
>>> +}
>>> +  free (gcov_file_name);
>>> +}
>>> +  fnotice (stdout, "\n");
>>> +}
>>>  }
>>>
>>> +  if (flag_gcov_file && flag_intermediate_format)
>>> +{
>>> +  /* Now we've finished writing the intermediate file.  */
>>> +  fclose (gcov_file_intermediate);
>>> +  XDELETEVEC (gcov_file_intermediate_name);
>>> +}
>>> +
>>>if (!file_name)
>>>  executed_summary (total_lines, total_executed);
>>>  }
>>> @@ -766,6 +914,9 @@ release_function (function_t *fn)
>>>  }
>>>free (fn->blocks);
>>>free (fn->counts);
>>> +  if (flag_demangled_names && fn->demangled_name != fn->name)
>>> +free (fn->demangled_name);
>>> +  free (fn->name);
>>>  }
>>>
>

Re: [patch] gcov intermediate format

2013-06-05 Thread Sharad Singhai
Ping.

Thanks,
Sharad


On Tue, May 28, 2013 at 11:35 AM, Sharad Singhai  wrote:
> Sorry, my patch had bad formatting in one of the functions
> (output_gcov_file). Here is the corrected version.
>
> Thanks,
> Sharad
>
> (2013-05-28
>
> * gcov.c (print_usage): Handle new option.
> (process_args): Ditto.
> (get_gcov_intermediate_filename): New function.
> (output_intermediate_file): New function.
> (output_gcov_file): New function
> (generate_results): Handle new option.
> (release_function): Relase demangled name.
> (read_graph_file): Handle demangled name.
> (output_lines): Ditto.
> * doc/gcov.texi: Document gcov intermediate format.
>
> testsuite/ChangeLog:
>
> 2013-05-28   Sharad Singhai  
>
> * g++.dg/gcov/gcov-8.C: New testcase.
> * lib/gcov.exp: Handle intermediate format.
>
> Index: doc/gcov.texi
> ===
> --- doc/gcov.texi (revision 199273)
> +++ doc/gcov.texi (working copy)
> @@ -122,15 +122,17 @@ gcov [@option{-v}|@option{--version}] [@option{-h}
>   [@option{-a}|@option{--all-blocks}]
>   [@option{-b}|@option{--branch-probabilities}]
>   [@option{-c}|@option{--branch-counts}]
> - [@option{-u}|@option{--unconditional-branches}]
> + [@option{-d}|@option{--display-progress}]
> + [@option{-f}|@option{--function-summaries}]
> + [@option{-i}|@option{--intermediate-format}]
> + [@option{-l}|@option{--long-file-names}]
> + [@option{-m}|@option{--demangled-names}]
>   [@option{-n}|@option{--no-output}]
> - [@option{-l}|@option{--long-file-names}]
> + [@option{-o}|@option{--object-directory} @var{directory|file}]
>   [@option{-p}|@option{--preserve-paths}]
>   [@option{-r}|@option{--relative-only}]
> - [@option{-f}|@option{--function-summaries}]
> - [@option{-o}|@option{--object-directory} @var{directory|file}]
>   [@option{-s}|@option{--source-prefix} @var{directory}]
> - [@option{-d}|@option{--display-progress}]
> + [@option{-u}|@option{--unconditional-branches}]
>   @var{files}
>  @c man end
>  @c man begin SEEALSO
> @@ -232,6 +234,50 @@ Unconditional branches are normally not interestin
>  @itemx --display-progress
>  Display the progress on the standard output.
>
> +@item -i
> +@itemx --intermediate-format
> +Output gcov file in an easy-to-parse intermediate text format that can
> +be used by @command{lcov} or other tools. The output is a single
> +@file{.gcov} file per @file{.gcda} file. No source code is required.
> +
> +The format of the intermediate @file{.gcov} file is plain text with
> +one entry per line
> +
> +@smallexample
> +file:@var{source_file_name}
> +function:@var{line_number},@var{execution_count},@var{function_name}
> +lcount:@var{line number},@var{execution_count}
> +branch:@var{line_number},@var{branch_coverage_type}
> +
> +Where the @var{branch_coverage_type} is
> +   notexec (Branch not executed)
> +   taken (Branch executed and taken)
> +   nottaken (Branch executed, but not taken)
> +
> +There can be multiple @var{file} entries in an intermediate gcov
> +file. All entries following a @var{file} pertain to that source file
> +until the next @var{file} entry.
> +@end smallexample
> +
> +Here is a sample when @option{-i} is used in conjuction with
> @option{-b} option:
> +
> +@smallexample
> +file:array.cc
> +function:11,1,_Z3sumRKSt6vectorIPiSaIS0_EE
> +function:22,1,main
> +lcount:11,1
> +lcount:12,1
> +lcount:14,1
> +branch:14,taken
> +lcount:26,1
> +branch:28,nottaken
> +@end smallexample
> +
> +@item -m
> +@itemx --demangled-names
> +Display demangled function names in output. The default is to show
> +mangled function names.
> +
>  @end table
>
>  @command{gcov} should be run with the current directory the same as that
> Index: gcov.c
> ===
> --- gcov.c (revision 199273)
> +++ gcov.c (working copy)
> @@ -37,6 +37,7 @@ along with Gcov; see the file COPYING3.  If not se
>  #include "intl.h"
>  #include "diagnostic.h"
>  #include "version.h"
> +#include "demangle.h"
>
>  #include 
>
> @@ -168,6 +169,7 @@ typedef struct function_info
>  {
>/* Name of function.  */
>char *name;
> +  char *demangled_name;
>unsigned ident;
>unsigned lineno_checksum;
>unsigned cfg_checksum;
> @@ -325,6 +327,14 @@ static int flag_gcov_file = 1;
>
>  static int flag_display_progress = 0;
>
> +/* Output *.gcov file in intermediate format used by 'lcov'.  */
> +
> +static int flag_intermediate_format = 0;
> +
> +/* Output demangle

[google/gcc-4_9] Fix static var promotion handling for LIPO

2014-09-15 Thread Sharad Singhai
This patch is for the google/gcc-4_9 branch to fix a LIPO issue.

This patch addresses handling for static initializer data during
static variable promotion in LIPO optimize phase. This is useful for
those targets which access initializer data via TOC, such as powerpc.

After static var promotion in two different modules, we have the
following which leads to multiple definition of ._41" error during
link time.

Module 1 Module 2
 .hidden ._41.cmo.1   .hidden ._41.cmo.3
 .globl ._41  .globl ._41
._41:._41:
  ...  ...


Instead we should use the appropriate unique names for initializer
data as in the following.

Module 1 Module 2
 .hidden ._41.cmo.1   .hidden ._41.cmo.3
 .globl ._41.cmo.1.globl ._41.cmo.3
._41.cmo.1:  ._41.cmo.3:
...  ...


Tested on powerpc as well as ran manual tests. Okay for google/4_9 branch?

Thanks,
Sharad
2014-09-15  Sharad Singhai  

Google Ref b/17114943

* l-ipo.c (promote_static_var_func): Update RTL with the unique name.

testsuite/ChangeLog:
* g++.dg/tree-prof/lipo/static1_0.C: New test.
* g++.dg/tree-prof/lipo/static1_1.C: New file.
* g++.dg/tree-prof/lipo/static1_2.C: New file.


Index: l-ipo.c
===
--- l-ipo.c (revision 215144)
+++ l-ipo.c (working copy)
@@ -47,6 +47,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "timevar.h"
 #include "vec.h"
 #include "params.h"
+#include "rtl.h"
+#include "varasm.h"
 
 unsigned ggc_total_memory; /* in KB */
 
@@ -1909,6 +1911,9 @@ promote_static_var_func (unsigned module_id, tree
 }
   varpool_link_node (node);
   insert_to_assembler_name_hash (node, false);
+  /* Possibly update the RTL name as well. */
+  if (DECL_RTL_SET_P (decl))
+XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (assemb_id);
 }
 
   if (is_extern)
Index: testsuite/g++.dg/tree-prof/lipo/static1_0.C
===
--- testsuite/g++.dg/tree-prof/lipo/static1_0.C (revision 0)
+++ testsuite/g++.dg/tree-prof/lipo/static1_0.C (working copy)
@@ -0,0 +1,48 @@
+// { dg-options "-std=c++11 -O2" }
+
+// A test case for static var promotion on targets like powerpc, where
+// the static initializer data is loaded via indirection through
+// TOC. Ensure that the global label for static initializer data is
+// unique via *.cmo* suffix.
+
+// Bug: after static var promotion in two different modules, we have
+// the following which leads to multiple definition of ._41" error during
+// link time.
+
+// Module 1 Module 2
+//  .hidden ._41.cmo.1   .hidden ._41.cmo.3
+//  .globl ._41  .globl ._41
+// ._41:._41:
+//   ...  ...
+
+
+// Instead we should use the appropriate unique names for initializer
+// data as in the following.
+
+// Module 1 Module 2
+//  .hidden ._41.cmo.1   .hidden ._41.cmo.3
+//  .globl ._41.cmo.1.globl ._41.cmo.3
+// ._41.cmo.1:  ._41.cmo.3:
+// ...  ...
+
+class A {
+ public:
+  int f(int x) const;
+};
+
+class B {
+ public:
+  int f(int x) const;
+};
+
+int main()
+{
+  A *a = new A();
+  B *b = new B();
+  int total = 0;
+  for (int i=0; i<3; ++i) {
+total += a->f(1);
+total += b->f(1);
+  }
+  return (total > 0) ? 0 : 1;
+}
Index: testsuite/g++.dg/tree-prof/lipo/static1_1.C
===
--- testsuite/g++.dg/tree-prof/lipo/static1_1.C (revision 0)
+++ testsuite/g++.dg/tree-prof/lipo/static1_1.C (working copy)
@@ -0,0 +1,14 @@
+/* { dg-options "-std=c++11 -O2" } */
+
+#include 
+
+class A {
+ public:
+  int f(int x) const;
+};
+
+static const std::vector point1_{42};
+
+int A::f(int x) const {
+  return x+1;
+}
Index: testsuite/g++.dg/tree-prof/lipo/static1_2.C
===
--- testsuite/g++.dg/tree-prof/lipo/static1_2.C (revision 0)
+++ testsuite/g++.dg/tree-prof/lipo/static1_2.C (working copy)
@@ -0,0 +1,14 @@
+/* { dg-options "-std=c++11 -O2" } */
+
+#include 
+
+class B {
+ public:
+  int f(int x) const;
+};
+
+static const std::vector point2_{43};
+
+int B::f(int x) const {
+  return x+1;
+}


Re: [patch] gcov intermediate format

2013-06-17 Thread Sharad Singhai
Ping.

Thanks,
Sharad


On Wed, Jun 5, 2013 at 11:18 AM, Sharad Singhai  wrote:
> Ping.
>
> Thanks,
> Sharad
>
>
> On Tue, May 28, 2013 at 11:35 AM, Sharad Singhai  wrote:
>> Sorry, my patch had bad formatting in one of the functions
>> (output_gcov_file). Here is the corrected version.
>>
>> Thanks,
>> Sharad
>>
>> (2013-05-28
>>
>> * gcov.c (print_usage): Handle new option.
>> (process_args): Ditto.
>> (get_gcov_intermediate_filename): New function.
>> (output_intermediate_file): New function.
>> (output_gcov_file): New function
>> (generate_results): Handle new option.
>> (release_function): Relase demangled name.
>> (read_graph_file): Handle demangled name.
>> (output_lines): Ditto.
>> * doc/gcov.texi: Document gcov intermediate format.
>>
>> testsuite/ChangeLog:
>>
>> 2013-05-28   Sharad Singhai  
>>
>> * g++.dg/gcov/gcov-8.C: New testcase.
>> * lib/gcov.exp: Handle intermediate format.
>>
>> Index: doc/gcov.texi
>> ===
>> --- doc/gcov.texi (revision 199273)
>> +++ doc/gcov.texi (working copy)
>> @@ -122,15 +122,17 @@ gcov [@option{-v}|@option{--version}] [@option{-h}
>>   [@option{-a}|@option{--all-blocks}]
>>   [@option{-b}|@option{--branch-probabilities}]
>>   [@option{-c}|@option{--branch-counts}]
>> - [@option{-u}|@option{--unconditional-branches}]
>> + [@option{-d}|@option{--display-progress}]
>> + [@option{-f}|@option{--function-summaries}]
>> + [@option{-i}|@option{--intermediate-format}]
>> + [@option{-l}|@option{--long-file-names}]
>> + [@option{-m}|@option{--demangled-names}]
>>   [@option{-n}|@option{--no-output}]
>> - [@option{-l}|@option{--long-file-names}]
>> + [@option{-o}|@option{--object-directory} @var{directory|file}]
>>   [@option{-p}|@option{--preserve-paths}]
>>   [@option{-r}|@option{--relative-only}]
>> - [@option{-f}|@option{--function-summaries}]
>> - [@option{-o}|@option{--object-directory} @var{directory|file}]
>>   [@option{-s}|@option{--source-prefix} @var{directory}]
>> - [@option{-d}|@option{--display-progress}]
>> + [@option{-u}|@option{--unconditional-branches}]
>>   @var{files}
>>  @c man end
>>  @c man begin SEEALSO
>> @@ -232,6 +234,50 @@ Unconditional branches are normally not interestin
>>  @itemx --display-progress
>>  Display the progress on the standard output.
>>
>> +@item -i
>> +@itemx --intermediate-format
>> +Output gcov file in an easy-to-parse intermediate text format that can
>> +be used by @command{lcov} or other tools. The output is a single
>> +@file{.gcov} file per @file{.gcda} file. No source code is required.
>> +
>> +The format of the intermediate @file{.gcov} file is plain text with
>> +one entry per line
>> +
>> +@smallexample
>> +file:@var{source_file_name}
>> +function:@var{line_number},@var{execution_count},@var{function_name}
>> +lcount:@var{line number},@var{execution_count}
>> +branch:@var{line_number},@var{branch_coverage_type}
>> +
>> +Where the @var{branch_coverage_type} is
>> +   notexec (Branch not executed)
>> +   taken (Branch executed and taken)
>> +   nottaken (Branch executed, but not taken)
>> +
>> +There can be multiple @var{file} entries in an intermediate gcov
>> +file. All entries following a @var{file} pertain to that source file
>> +until the next @var{file} entry.
>> +@end smallexample
>> +
>> +Here is a sample when @option{-i} is used in conjuction with
>> @option{-b} option:
>> +
>> +@smallexample
>> +file:array.cc
>> +function:11,1,_Z3sumRKSt6vectorIPiSaIS0_EE
>> +function:22,1,main
>> +lcount:11,1
>> +lcount:12,1
>> +lcount:14,1
>> +branch:14,taken
>> +lcount:26,1
>> +branch:28,nottaken
>> +@end smallexample
>> +
>> +@item -m
>> +@itemx --demangled-names
>> +Display demangled function names in output. The default is to show
>> +mangled function names.
>> +
>>  @end table
>>
>>  @command{gcov} should be run with the current directory the same as that
>> Index: gcov.c
>> ===
>> --- gcov.c (revision 199273)
>> +++ gcov.c (working copy)
>> @@ -37,6 +37,7 @@ along with Gcov; see the file COPYING3.  If not se
>>  #include "intl.h"
>>  #include "diagnostic.h"
>>  #include "version.h"
>

Re: [patch] gcov intermediate format

2013-06-18 Thread Sharad Singhai
On Tue, Jun 18, 2013 at 3:28 AM, Jan Hubicka  wrote:
>> Ping.
> Th patch is OK, thanks!
> I see you added gcov.exp file support, do you have a testcases?

Yes, I added support for verifying intermediate format in gcov.exp. I
also added a minimal testcase for intermediate format in
testsuite/g++.dg/gcov directory.

Thanks,
Sharad

>
> Honza


[google/gcc-4_8] Port -Wreal-conversion warning

2013-06-24 Thread Sharad Singhai
Hi,

This patch forward ports the -Wreal-conversion warning to
google/gcc-4_8 branch (google ref 39133-p2). I tweaked the patch a
little bit to avoid printing duplicate warnings when both -Wconversion
and -Wreal-conversion are specified. Also, I trimmed the test case to
avoid testing for integer-to-integer narrowing conversions as this
warning is about real conversions.

Bootstrapped and tested on x86_64. Okay for google/gcc-4_8?

Thanks,
Sharad

2013-06-24

* doc/invoke.texi: Document new option -Wreal-conversion.
* c-family/c.opt: Handle new option.
* c-family/c-opts.c (c_common_post_options): Ditto.
* c-family/c-common.c (conversion_warning): Ditto.

testsuite/ChangeLog:

* testsuite/gcc.dg/Wreal-conversion-1.c: New test.
* testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.

Index: testsuite/gcc.dg/Wreal-conversion-1.c
===
--- testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
+++ testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-Wreal-conversion" }
+
+#include 
+
+int func1(int a) {
+  double f = a;
+  return f;   // { dg-warning "conversion to" }
+}
+
+double func3();
+
+void func2() {
+  double g = 3.2;
+  float f;
+  int t = g;  // { dg-warning "conversion to" }
+  int p;
+  p = f;  // { dg-warning "conversion to" }
+  func1(g);   // { dg-warning "conversion to" }
+  char c = f; // { dg-warning "conversion to" }
+  int q;
+  q = func3();// { dg-warning "conversion to" }
+}
Index: testsuite/g++.dg/warn/Wreal-conversion-1.C
===
--- testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
+++ testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
@@ -0,0 +1,24 @@
+// { dg-do compile }
+// { dg-options "-Wreal-conversion" }
+
+#include 
+
+int func1(int a) {
+  double f = a;
+  return f;   // { dg-warning "conversion to" }
+}
+
+double func3();
+
+void func2() {
+  double g = 3.2;
+  float f;
+  int t = g;  // { dg-warning "conversion to" }
+  bool b = g;
+  int p;
+  p = f;  // { dg-warning "conversion to" }
+  func1(g);   // { dg-warning "conversion to" }
+  char c = f; // { dg-warning "conversion to" }
+  int q;
+  q = func3();// { dg-warning "conversion to" }
+}
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 200359)
+++ doc/invoke.texi (working copy)
@@ -193,7 +193,7 @@ in the following sections.
 -fno-default-inline  -fvisibility-inlines-hidden @gol
 -fvisibility-ms-compat @gol
 -fext-numeric-literals @gol
--Wabi  -Wconversion-null  -Wctor-dtor-privacy @gol
+-Wabi  -Wconversion-null -Wctor-dtor-privacy @gol
 -Wdelete-non-virtual-dtor -Wliteral-suffix -Wnarrowing @gol
 -Wnoexcept -Wnon-virtual-dtor  -Wreorder @gol
 -Weffc++  -Wstrict-null-sentinel @gol
@@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment @gol
--Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
+-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
-Wno-deprecated  @gol
 -Wno-deprecated-declarations -Wdisabled-optimization  @gol
 -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
 -Wno-endif-labels -Werror  -Werror=* @gol
@@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
 unsigned integers are disabled by default in C++ unless
 @option{-Wsign-conversion} is explicitly enabled.

+@item -Wreal-conversion
+@opindex Wreal-conversion
+@opindex Wno-real-conversion
+Warn for implicit type conversions from real (@code{double} or @code{float})
+to integral values.
+
 @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
 @opindex Wconversion-null
 @opindex Wno-conversion-null
Index: c-family/c.opt
===
--- c-family/c.opt (revision 200359)
+++ c-family/c.opt (working copy)
@@ -677,6 +677,10 @@ Wsign-compare
 C ObjC C++ ObjC++ EnabledBy(Wextra)
 ;

+Wreal-conversion
+C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
+Warn for implicit type conversions from real to integral values
+
 Wsign-conversion
 C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
 Warn for implicit type conversions between signed and unsigned integers
Index: c-family/c-opts.c
===
--- c-family/c-opts.c (revision 200359)
+++ c-family/c-opts.c (working copy)
@@ -876,6 +876,12 @@ c_common_post_options (const char **pfilename)
   if (warn_packed_bitfield_compat == -1)
 warn_packed_bitfield_compat = 1;

+  /* Enable warning for converting real values to integral values
+ when -Wconversion is specified (unless disabled 

Re: [google/gcc-4_8] Port -Wreal-conversion warning

2013-06-24 Thread Sharad Singhai
On Mon, Jun 24, 2013 at 9:14 PM, Xinliang David Li  wrote:
> To avoid printing twice, can you just do
>
> opt_type = (warn_conversion ? OPT_Wconversion : OPT_Wreal_conversion);
> warning_at (loc, opt_type, ...);

Thanks for the suggestion. I have updated the enclosed patch and
retested. Okay for google/gcc-4_8?

Thanks,
Sharad

2013-06-24

* doc/invoke.texi: Document new option -Wreal-conversion.
* c-family/c.opt: Handle new option.
* c-family/c-opts.c (c_common_post_options): Ditto.
* c-family/c-common.c (conversion_warning): Ditto.

testsuite/ChangeLog:

* testsuite/gcc.dg/Wreal-conversion-1.c: New test.
* testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 200359)
+++ doc/invoke.texi (working copy)
@@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment @gol
--Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
+-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
-Wno-deprecated  @gol
 -Wno-deprecated-declarations -Wdisabled-optimization  @gol
 -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
 -Wno-endif-labels -Werror  -Werror=* @gol
@@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
 unsigned integers are disabled by default in C++ unless
 @option{-Wsign-conversion} is explicitly enabled.

+@item -Wreal-conversion
+@opindex Wreal-conversion
+@opindex Wno-real-conversion
+Warn for implicit type conversions from real (@code{double} or @code{float})
+to integral values.
+
 @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
 @opindex Wconversion-null
 @opindex Wno-conversion-null
Index: testsuite/gcc.dg/Wreal-conversion-1.c
===
--- testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
+++ testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-Wreal-conversion" }
+
+#include 
+
+int func1(int a) {
+  double f = a;
+  return f;   // { dg-warning "conversion to" }
+}
+
+double func3();
+
+void func2() {
+  double g = 3.2;
+  float f;
+  int t = g;  // { dg-warning "conversion to" }
+  int p;
+  p = f;  // { dg-warning "conversion to" }
+  func1(g);   // { dg-warning "conversion to" }
+  char c = f; // { dg-warning "conversion to" }
+  int q;
+  q = func3();// { dg-warning "conversion to" }
+}
Index: testsuite/g++.dg/warn/Wreal-conversion-1.C
===
--- testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
+++ testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
@@ -0,0 +1,24 @@
+// { dg-do compile }
+// { dg-options "-Wreal-conversion" }
+
+#include 
+
+int func1(int a) {
+  double f = a;
+  return f;   // { dg-warning "conversion to" }
+}
+
+double func3();
+
+void func2() {
+  double g = 3.2;
+  float f;
+  int t = g;  // { dg-warning "conversion to" }
+  bool b = g;
+  int p;
+  p = f;  // { dg-warning "conversion to" }
+  func1(g);   // { dg-warning "conversion to" }
+  char c = f; // { dg-warning "conversion to" }
+  int q;
+  q = func3();// { dg-warning "conversion to" }
+}
Index: c-family/c.opt
===
--- c-family/c.opt (revision 200359)
+++ c-family/c.opt (working copy)
@@ -677,6 +677,10 @@ Wsign-compare
 C ObjC C++ ObjC++ EnabledBy(Wextra)
 ;

+Wreal-conversion
+C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
+Warn for implicit type conversions from real to integral values
+
 Wsign-conversion
 C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
 Warn for implicit type conversions between signed and unsigned integers
Index: c-family/c-opts.c
===
--- c-family/c-opts.c (revision 200359)
+++ c-family/c-opts.c (working copy)
@@ -876,6 +876,12 @@ c_common_post_options (const char **pfilename)
   if (warn_packed_bitfield_compat == -1)
 warn_packed_bitfield_compat = 1;

+  /* Enable warning for converting real values to integral values
+ when -Wconversion is specified (unless disabled through
+ -Wno-real-conversion).  */
+  if (warn_real_conversion == -1)
+warn_real_conversion = warn_conversion;
+
   /* Special format checking options don't work without -Wformat; warn if
  they are used.  */
   if (!warn_format)
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 200359)
+++ c-family/c-common.c (working copy)
@@ -2668,7 +2668,7 @@ conversion_warning (tree type, tree expr)

Re: [google/gcc-4_8] Port -Wreal-conversion warning

2013-06-26 Thread Sharad Singhai
Sorry, my port was bad. I am going to revert this patch and redo it.

Thanks,
Sharad

On Wed, Jun 26, 2013 at 4:03 PM, Xinliang David Li  wrote:
> The warn_type should also be guarded with float type check:
>
>  warn_type = (warn_real_conversion
> && (FLOAT_TYPE_P (type) || FLOAT_TYPE_P (expr_type)))
> ? OPT_Wreal_conversion
> : OPT_Wconversion;
>
> Also why did you put the warn_type code inside the default?
>
> David
>
>
> On Mon, Jun 24, 2013 at 10:03 PM, Sharad Singhai  wrote:
>> On Mon, Jun 24, 2013 at 9:14 PM, Xinliang David Li  
>> wrote:
>>> To avoid printing twice, can you just do
>>>
>>> opt_type = (warn_conversion ? OPT_Wconversion : OPT_Wreal_conversion);
>>> warning_at (loc, opt_type, ...);
>>
>> Thanks for the suggestion. I have updated the enclosed patch and
>> retested. Okay for google/gcc-4_8?
>>
>> Thanks,
>> Sharad
>>
>> 2013-06-24
>>
>> * doc/invoke.texi: Document new option -Wreal-conversion.
>> * c-family/c.opt: Handle new option.
>> * c-family/c-opts.c (c_common_post_options): Ditto.
>> * c-family/c-common.c (conversion_warning): Ditto.
>>
>> testsuite/ChangeLog:
>>
>> * testsuite/gcc.dg/Wreal-conversion-1.c: New test.
>> * testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.
>>
>> Index: doc/invoke.texi
>> ===
>> --- doc/invoke.texi (revision 200359)
>> +++ doc/invoke.texi (working copy)
>> @@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
>>  -Wno-attributes -Wno-builtin-macro-redefined @gol
>>  -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
>>  -Wchar-subscripts -Wclobbered  -Wcomment @gol
>> --Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
>> +-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
>> -Wno-deprecated  @gol
>>  -Wno-deprecated-declarations -Wdisabled-optimization  @gol
>>  -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
>>  -Wno-endif-labels -Werror  -Werror=* @gol
>> @@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
>>  unsigned integers are disabled by default in C++ unless
>>  @option{-Wsign-conversion} is explicitly enabled.
>>
>> +@item -Wreal-conversion
>> +@opindex Wreal-conversion
>> +@opindex Wno-real-conversion
>> +Warn for implicit type conversions from real (@code{double} or @code{float})
>> +to integral values.
>> +
>>  @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
>>  @opindex Wconversion-null
>>  @opindex Wno-conversion-null
>> Index: testsuite/gcc.dg/Wreal-conversion-1.c
>> ===
>> --- testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
>> +++ testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
>> @@ -0,0 +1,23 @@
>> +// { dg-do compile }
>> +// { dg-options "-Wreal-conversion" }
>> +
>> +#include 
>> +
>> +int func1(int a) {
>> +  double f = a;
>> +  return f;   // { dg-warning "conversion to" }
>> +}
>> +
>> +double func3();
>> +
>> +void func2() {
>> +  double g = 3.2;
>> +  float f;
>> +  int t = g;  // { dg-warning "conversion to" }
>> +  int p;
>> +  p = f;  // { dg-warning "conversion to" }
>> +  func1(g);   // { dg-warning "conversion to" }
>> +  char c = f; // { dg-warning "conversion to" }
>> +  int q;
>> +  q = func3();// { dg-warning "conversion to" }
>> +}
>> Index: testsuite/g++.dg/warn/Wreal-conversion-1.C
>> ===
>> --- testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
>> +++ testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
>> @@ -0,0 +1,24 @@
>> +// { dg-do compile }
>> +// { dg-options "-Wreal-conversion" }
>> +
>> +#include 
>> +
>> +int func1(int a) {
>> +  double f = a;
>> +  return f;   // { dg-warning "conversion to" }
>> +}
>> +
>> +double func3();
>> +
>> +void func2() {
>> +  double g = 3.2;
>> +  float f;
>> +  int t = g;  // { dg-warning "conversion to" }
>> +  bool b = g;
>> +  int p;
>> +  p = f;  // { dg-warning "conversion to" }
>> +  func1(g);   // 

Re: [google/gcc-4_8] Port -Wreal-conversion warning

2013-06-26 Thread Sharad Singhai
I reverted the earlier broken patch. I am including an updated patch
which warns only for real conversion, not for integral conversions. I
also updated the test case to include an integral conversion (int to
char) which doesn't emit the warning with the -Wreal-conversion flag.

Bootstrapped and tested on x86_64. Okay for google/gcc-4_8?

Thanks,
Sharad

2013-06-26

* doc/invoke.texi: Document new option -Wreal-conversion.
* c-family/c.opt: Handle new option.
* c-family/c-opts.c (c_common_post_options): Ditto.
* c-family/c-common.c (conversion_warning): Ditto.

testsuite/ChangeLog:

* testsuite/gcc.dg/Wreal-conversion-1.c: New test.
* testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 200448)
+++ doc/invoke.texi (working copy)
@@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment @gol
--Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
+-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
-Wno-deprecated  @gol
 -Wno-deprecated-declarations -Wdisabled-optimization  @gol
 -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
 -Wno-endif-labels -Werror  -Werror=* @gol
@@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
 unsigned integers are disabled by default in C++ unless
 @option{-Wsign-conversion} is explicitly enabled.

+@item -Wreal-conversion
+@opindex Wreal-conversion
+@opindex Wno-real-conversion
+Warn for implicit type conversions from real (@code{double} or @code{float})
+to integral values. This warning is also enabled by @option{-Wconversion}.
+
 @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
 @opindex Wconversion-null
 @opindex Wno-conversion-null
Index: c-family/c.opt
===
--- c-family/c.opt (revision 200448)
+++ c-family/c.opt (working copy)
@@ -677,6 +677,10 @@ Wsign-compare
 C ObjC C++ ObjC++ EnabledBy(Wextra)
 ;

+Wreal-conversion
+C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
EnabledBy(Wconversion)
+Warn for implicit type conversions from real to integral values
+
 Wsign-conversion
 C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
 Warn for implicit type conversions between signed and unsigned integers
Index: c-family/c-opts.c
===
--- c-family/c-opts.c (revision 200448)
+++ c-family/c-opts.c (working copy)
@@ -876,6 +876,12 @@ c_common_post_options (const char **pfilename)
   if (warn_packed_bitfield_compat == -1)
 warn_packed_bitfield_compat = 1;

+  /* Enable warning for converting real values to integral values
+ when -Wconversion is specified (unless disabled through
+ -Wno-real-conversion).  */
+  if (warn_real_conversion == -1)
+warn_real_conversion = warn_conversion;
+
   /* Special format checking options don't work without -Wformat; warn if
  they are used.  */
   if (!warn_format)
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 200448)
+++ c-family/c-common.c (working copy)
@@ -2665,12 +2665,22 @@ unsafe_conversion_p (tree type, tree expr, bool pr
 static void
 conversion_warning (tree type, tree expr)
 {
+  int warn_option;
   tree expr_type = TREE_TYPE (expr);
   location_t loc = EXPR_LOC_OR_HERE (expr);

-  if (!warn_conversion && !warn_sign_conversion)
+  if (!warn_conversion && !warn_sign_conversion && !warn_real_conversion)
 return;

+  /* When either type is a floating point type, warn with
+ -Wreal-conversion instead of -Wconversion (-Wreal-conversion is a
+ subset of -Wconversion that only warns for conversions of real
+ types to integral types).  */
+  warn_option = (warn_real_conversion
+ && (FLOAT_TYPE_P (type) || FLOAT_TYPE_P (expr_type)))
+? OPT_Wreal_conversion
+: OPT_Wconversion;
+
   switch (TREE_CODE (expr))
 {
 case EQ_EXPR:
@@ -2689,14 +2699,14 @@ conversion_warning (tree type, tree expr)
  can hold the values 0 and -1) doesn't lose information - but
  it does change the value.  */
   if (TYPE_PRECISION (type) == 1 && !TYPE_UNSIGNED (type))
- warning_at (loc, OPT_Wconversion,
+ warning_at (loc, warn_option,
 "conversion to %qT from boolean expression", type);
   return;

 case REAL_CST:
 case INTEGER_CST:
   if (unsafe_conversion_p (type, expr, true))
- warning_at (loc, OPT_Wconversion,
+ warning_at (loc, warn_option,
 "conversion to %qT alters %qT constant value",
 type, expr_type);
   return;
@@ -2715,7 +2725,7 @@ conversion_warning (tree type, tree expr)

 default: /* 'expr' is not a constant.  */
   if (unsafe_co

Re: [PATCH] Convert more passes to new dump framework

2013-08-06 Thread Sharad Singhai
On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  wrote:
> On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>> Hi,
>>
>> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>>> This patch ports messages to the new dump framework,
>>
>> It would be great this new framework was documented somewhere.  I lost
>> track of what was agreed it would be and from the uses in the
>> vectorizer I was never quite sure how to utilize it in other passes.
>
> Sharad, can you put the documentation in GCC wiki.

Sure. I had user documentation in form of gcc info. But I will add
more developer details to a GCC wiki.

Thanks,
Sharad

> In a nutshell, the new dumping interfaces produces information notes
> which have 'dual' outputs -- controlled by different options. When
> -fdump-- is on, the dump info will be dumped into the
> pass specific dump file, and when -fopt-info=.. is on, the information
> will be dumped into stderr.
>
> The dump call should be guarded by dump_enabled_p().
>
> thanks,
>
> David
>
>>
>> I'd also like to point out two other minor things inline:
>>
>> [...]
>>
>>> 2013-08-06  Teresa Johnson  
>>> Dehao Chen  
>>>
>>> * dumpfile.c (dump_loc): Add column number to output, make newlines
>>> consistent.
>>> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>>> * ipa-inline-transform.c (clone_inlined_nodes):
>>> (cgraph_node_opt_info): New function.
>>> (cgraph_node_call_chain): Ditto.
>>> (dump_inline_decision): Ditto.
>>> (inline_call): Invoke dump_inline_decision.
>>> * doc/invoke.texi: Document optall -fopt-info flag.
>>> * profile.c (read_profile_edge_counts): Use new dump framework.
>>> (compute_branch_probabilities): Ditto.
>>> * passes.c (pass_manager::register_one_dump_file): Use 
>>> OPTGROUP_OTHER
>>> when pass not in any opt group.
>>> * value-prof.c (check_counter): Use new dump framework.
>>> (find_func_by_funcdef_no): Ditto.
>>> (check_ic_target): Ditto.
>>> * coverage.c (get_coverage_counts): Ditto.
>>> (coverage_init): Setup new dump framework.
>>> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>>> * ipa-inline.h (is_in_ipa_inline): Declare.
>>>
>>> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>>> * testsuite/gcc.dg/pr26570.c: Ditto.
>>> * testsuite/gcc.dg/pr32773.c: Ditto.
>>> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>>>
>>
>> [...]
>>
>>> Index: ipa-inline-transform.c
>>> ===
>>> --- ipa-inline-transform.c  (revision 201461)
>>> +++ ipa-inline-transform.c  (working copy)
>>> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>>>  }
>>>
>>>
>>> +#define MAX_INT_LENGTH 20
>>> +
>>> +/* Return NODE's name and profile count, if available.  */
>>> +
>>> +static const char *
>>> +cgraph_node_opt_info (struct cgraph_node *node)
>>> +{
>>> +  char *buf;
>>> +  size_t buf_size;
>>> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>>> +
>>> +  if (!bfd_name)
>>> +bfd_name = "unknown";
>>> +
>>> +  buf_size = strlen (bfd_name) + 1;
>>> +  if (profile_info)
>>> +buf_size += (MAX_INT_LENGTH + 3);
>>> +
>>> +  buf = (char *) xmalloc (buf_size);
>>> +
>>> +  strcpy (buf, bfd_name);
>>> +
>>> +  if (profile_info)
>>> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>>> +  return buf;
>>> +}
>>
>> I'm not sure if output of this function is aimed only at the user or
>> if it is supposed to be used by gcc developers as well.  If the
>> latter, an incredibly useful thing is to also dump node->symbol.order
>> too.  We usually dump it after "/" sign separating it from node name.
>> It is invaluable when examining decisions in C++ code where you can
>> have lots of clones of a node (and also because existing dumps print
>> it, it is easy to combine them).
>>
>> [...]
>>
>>> Index: ipa-inline.c
>>> ===
>>> --- ipa-inline.c(revision 201461)
>>> +++ ipa-inline.c(working copy)
>>> @@ -118,6 +118,9 @@ along with GCC; see the file COPYING3.  If not see
>>>  static int overall_size;
>>>  static gcov_type max_count;
>>>
>>> +/* Global variable to denote if it is in ipa-inline pass. */
>>> +bool is_in_ipa_inline = false;
>>> +
>>>  /* Return false when inlining edge E would lead to violating
>>> limits on function unit growth or stack usage growth.
>>>
>>
>> In this age of removing global variables, are you sure you need this?
>> The only user of this seems to be a function that is only being called
>> from inline_call... can that ever happen when not inlining?  If you
>> plan to use this function also elsewhere, perhaps the callers will
>> know whether we are inlining or not and can provide this in a
>> parameter?
>>
>>

Re: [PATCH] Convert more passes to new dump framework

2013-08-06 Thread Sharad Singhai
On Tue, Aug 6, 2013 at 10:10 AM, Martin Jambor  wrote:
> On Tue, Aug 06, 2013 at 09:22:02AM -0700, Sharad Singhai wrote:
>> On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  wrote:
>> > On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>> >> Hi,
>> >>
>> >> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> >>> This patch ports messages to the new dump framework,
>> >>
>> >> It would be great this new framework was documented somewhere.  I lost
>> >> track of what was agreed it would be and from the uses in the
>> >> vectorizer I was never quite sure how to utilize it in other passes.
>> >
>> > Sharad, can you put the documentation in GCC wiki.
>>
>> Sure. I had user documentation in form of gcc info. But I will add
>> more developer details to a GCC wiki.
>>
>
> I have built trunk gccint.info yesterday but could not find any string
> dump_enabled_p there, for example.  And when I quickly searched just
> for the string "dump," I did not find any thing that looked like
> dumping infrastructure either.  OTOH, I agree that fie would be the
> best place for the documentation.
>
> Or did I just miss it?  What section is it in then?

Actually, the user-facing documentation is in doc/invoke.texi.
However, that doesn't describe dump_enabled_p. Do you think
gccint.info would be a good place? I can add documentation there
instead of creating a GCC wiki.

Thanks,
Sharad

> Thanks,
>
> Martin
>


Re: [PATCH] Documentation for dump and optinfo output

2014-02-05 Thread Sharad Singhai
I am really sorry about the delay. I couldn't exactly reproduce the
warning which you described, but I found a place where two nodes were
out of order in optinfo.texi. Could you please apply the following
patch and see if the problem goes away? If it works for you, I will
commit the doc fixes.

Also I would appreciate the exact command which produces these warnings.

Thanks,
Sharad

2014-02-05  Sharad Singhai  

* doc/optinfo.texi: Fix order of nodes.

Index: doc/optinfo.texi
===
--- doc/optinfo.texi (revision 207525)
+++ doc/optinfo.texi (working copy)
@@ -12,8 +12,8 @@ information about various compiler transformations
 @menu
 * Dump setup:: Setup of optimization dumps.
 * Optimization groups::Groups made up of optimization passes.
+* Dump files and streams:: Dump output file names and streams.
 * Dump output verbosity::  How much information to dump.
-* Dump files and streams:: Dump output file names and streams.
 * Dump types:: Various types of dump functions.
 * Dump examples::  Sample usage.
 @end menu

Thanks,
Sharad


On Fri, Jan 24, 2014 at 2:20 AM, Thomas Schwinge
 wrote:
> Hi!
>
> On Thu, 19 Dec 2013 01:44:08 -0800, Sharad Singhai  wrote:
>> I ran 'make doc' and browsed the info output for basic sanity. Okay for 
>> trunk?
>
>>   * Makefile.in: Add optinfo.texi.
>>   * doc/optinfo.texi: New documentation for optimization info.
>>   * doc/passes.texi: Add new node.
>
> Thanks for contributing to the documentation!
>
> During build, I'm seeing the following warnings; would be great if you
> could address these:
>
> ../../source/gcc/doc/optinfo.texi:45: warning: node next `Optimization 
> groups' in menu `Dump output verbosity' and in sectioning `Dump files and 
> streams' differ
> ../../source/gcc/doc/optinfo.texi:77: warning: node next `Dump files and 
> streams' in menu `Dump types' and in sectioning `Dump output verbosity' differ
> ../../source/gcc/doc/optinfo.texi:77: warning: node prev `Dump files and 
> streams' in menu `Dump output verbosity' and in sectioning `Optimization 
> groups' differ
> ../../source/gcc/doc/optinfo.texi:104: warning: node next `Dump output 
> verbosity' in menu `Dump files and streams' and in sectioning `Dump types' 
> differ
> ../../source/gcc/doc/optinfo.texi:104: warning: node prev `Dump output 
> verbosity' in menu `Optimization groups' and in sectioning `Dump files and 
> streams' differ
> ../../source/gcc/doc/optinfo.texi:137: warning: node prev `Dump types' in 
> menu `Dump files and streams' and in sectioning `Dump output verbosity' differ
>
>
> Grüße,
>  Thomas


Re: [PATCH] Documentation for dump and optinfo output

2014-02-13 Thread Sharad Singhai
Committed as r207767.

Indeed, I had an older version of makeinfo. Once I updated to the
latest version 5.2, I saw the warnings. Those are fixed by this patch.

Thanks,
Sharad

> On Tue, Feb 11, 2014 at 11:42 PM, Thomas Schwinge 
> wrote:
>>
>> Hi!
>>
>> On Wed, 5 Feb 2014 16:33:19 -0800, Sharad Singhai 
>> wrote:
>> > I am really sorry about the delay.
>>
>> No worries; not exactly a severe issue.  ;-)
>>
>> > I couldn't exactly reproduce the
>> > warning which you described
>>
>> Maybe the version of makeinfo is relevant?
>>
>> $ makeinfo --version | head -n 1
>> makeinfo (GNU texinfo) 5.1
>>
>> > but I found a place where two nodes were
>> > out of order in optinfo.texi. Could you please apply the following
>> > patch and see if the problem goes away? If it works for you, I will
>> > commit the doc fixes.
>> >
>> > Also I would appreciate the exact command which produces these warnings.
>>
>> Your patch does fix the problem; see the following diff of the build log,
>> where the warnings are now gone, and which also happens to contain the
>> makeinfo command line.
>>
>> @@ -4199,12 +4199,6 @@ if [ xinfo = xinfo ]; then \
>> makeinfo --split-size=500 --split-size=500
>> --no-split -I . -I ../../source/gcc/doc \
>> -I ../../source/gcc/doc/include -o doc/gccint.info
>> ../../source/gcc/doc/gccint.texi; \
>> fi
>> -../../source/gcc/doc/optinfo.texi:45: warning: node next `Optimization
>> groups' in menu `Dump output verbosity' and in sectioning `Dump files and
>> streams' differ
>> -../../source/gcc/doc/optinfo.texi:77: warning: node next `Dump files and
>> streams' in menu `Dump types' and in sectioning `Dump output verbosity'
>> differ
>> -../../source/gcc/doc/optinfo.texi:77: warning: node prev `Dump files and
>> streams' in menu `Dump output verbosity' and in sectioning `Optimization
>> groups' differ
>> -../../source/gcc/doc/optinfo.texi:104: warning: node next `Dump output
>> verbosity' in menu `Dump files and streams' and in sectioning `Dump types'
>> differ
>> -../../source/gcc/doc/optinfo.texi:104: warning: node prev `Dump output
>> verbosity' in menu `Optimization groups' and in sectioning `Dump files and
>> streams' differ
>> -../../source/gcc/doc/optinfo.texi:137: warning: node prev `Dump types' in
>> menu `Dump files and streams' and in sectioning `Dump output verbosity'
>> differ
>>  if [ xinfo = xinfo ]; then \
>> makeinfo --split-size=500 --split-size=500
>> --no-split -I ../../source/gcc/doc \
>> -I ../../source/gcc/doc/include -o
>> doc/gccinstall.info ../../source/gcc/doc/install.texi; \
>>
>> > * doc/optinfo.texi: Fix order of nodes.
>>
>> Thanks, please commit.
>>
>>
>> Grüße,
>>  Thomas
>
>


[PATCH] Use new dump scheme for loop unroll passes

2012-12-13 Thread Sharad Singhai
Hi,

As per discussion in http://gcc.gnu.org/ml/gcc/2012-12/msg00056.html,
the attached patch updates loop unroll passes to use new dump
infrastructure.

This patch filters relevant dump messages into the following
three categories

- optimized: an optimization was successfully applied
- missed: an optimization was missed due to the described reason
- note: other relevant/detailed info during optimization. For example,
  loop unrolling computes the loop bounds and size.

Two sample outputs from one of the gcc tests (gcc.dg/unroll_1.c) are below.

Sample 1
-- info about optimized loops via
"-fopt-info-loop-optimized" ---
$ gcc gcc.dg/unroll_1.c -fno-diagnostics-show-caret -O2 -S
-fdump-rtl-loop2_unroll -funroll-loops -fopt-info-loop-optimized

Unrolled loop 1 completely (duplicated 2 times).
Exit condition of peeled iterations was eliminated.
Last iteration exit edge was proved true.
Unrolled loop 1 completely (duplicated 2 times).
Exit condition of peeled iterations was eliminated.
Last iteration exit edge was proved true.


Sample 2:
--- All available loop optimization info, i.e., optimized+missed+note
via "-fopt-info-loop" ---
$ gcc gcc.dg/unroll_1.c -fno-diagnostics-show-caret -O2 -S
-fdump-rtl-loop2_unroll -funroll-loops -fopt-info-loop

Loop 1 iterates 2 times.
Loop 1 iterates at most 2 times.
Estimating sizes for loop 1
 BB: 4, after_exit: 0
  size:   2 if (i_1 <= 1)
   Exit condition will be eliminated in peeled copies.
 BB: 3, after_exit: 1
  size:   1 _5 = b[i_1];
  size:   1 _6 = _5 + 1;
  size:   1 a[i_1] = _6;
  size:   1 i_8 = i_1 + 1;
   Induction variable computation will be folded away.
size: 6-3, last_iteration: 2-0
  Loop size: 6
  Estimated size after unrolling: 5
Unrolled loop 1 completely (duplicated 2 times).
Exit condition of peeled iterations was eliminated.
Last iteration exit edge was proved true.
Forced exit to be taken: if (1 == 0)
Loop 1 iterates 2 times.
Loop 1 iterates at most 2 times.
Estimating sizes for loop 1
 BB: 4, after_exit: 0
  size:   2 if (i_1 <= 1)
   Exit condition will be eliminated in peeled copies.
 BB: 3, after_exit: 1
  size:   1 _4 = b[i_1];
  size:   1 _5 = _4 + 1;
  size:   1 a[i_1] = _5;
  size:   1 i_7 = i_1 + 1;
   Induction variable computation will be folded away.
size: 6-3, last_iteration: 2-0
  Loop size: 6
  Estimated size after unrolling: 5
Unrolled loop 1 completely (duplicated 2 times).
Exit condition of peeled iterations was eliminated.
Last iteration exit edge was proved true.
Forced exit to be taken: if (1 == 0)


I would like to mention that this information is perhaps too verbose
and the the source location of optimized loops is not displayed. I can
add source line info (and fix up corresponding tests) if needed. But
right now I wanted to maintain current dump format faithfully. Perhaps
the format can be tweaked for better readability.

Note that all information dumped in response to -fopt-info is also
present in regular dump file(s) when corresponding dumps are
enabled. Thus in above examples, the loop optimization info is also
present in *.loop2_unroll dump file since the command line specified a
dump file via "-fdump-rtl-loop2_unroll" in addition to -fopt-info.

(As a side note, while doing the conversion, I found that the MSG_*
dump flags are unwieldy when used in conjunction with other
flags. Perhaps these flags should be renamed/shortened. I propose the following
   MSG_MISSED_OPTIMIZATION  ==> MSG_MISSED
   MSG_OPTIMIZED_LOCATIONS  ==> MSG_OPTIMIZED
But that is pure renaming and can be done separately.)

I have bootstrapped and tested this patch on x86_64 and found no new
failures. Okay for trunk?

Thanks,
Sharad


unroll.opt_info.patch
Description: Binary data


Re: [PATCH] Use new dump scheme for loop unroll passes

2012-12-14 Thread Sharad Singhai
Teresa,

Yes, I didn't take enhancements in google branches into account while
porting this patch. In light of these comments, I withdraw this patch
and will wait for your patch. Once your patch is in, I will update
this patch for regular dumps.

To answer your other question, yes, the new dump infrastructure can
dump to either dump file or opt-info streams (or both) depending upon
dump_flags. If the dump_flags contain TDF_* flags then the dump
happens on regular dump files, if dump_flags contain MSG_* flags then
the dump happens on opt-info stream.

Thanks,
Sharad


On Thu, Dec 13, 2012 at 9:37 PM, Teresa Johnson  wrote:
> On Thu, Dec 13, 2012 at 8:58 PM, Xinliang David Li  wrote:
>> A couple of comments:
>>
>> 1) please dump with source location if possible. What is the use of
>> the information if there is no line number
>
> The google branches have the code to identify a source location of the
> loop, and a similar message to the one proposed below (which uses the
> inform() interface on the google branches). I have a trunk patch ready
> to submit with this ported to the new dumping infrastructure, which I
> was going to submit after Sharad's patch. Sharad, do you want me to
> submit that one first, then it can be leveraged if you want to extend
> the messages? But I agree with David in that I think the bulk of these
> types of messages should stay in the dump file and not be emitted by
> -fopt-info because they are too verbose and low-level. Can the new
> dumping infrastructure be used to just dump to the dump file and not
> via -fopt-info?
>
> Teresa
>
>> 2) Please do not use the existing dump report -- Loop 1,2,3 etc means
>> nothing to user
>> 3) The optimization report should be standardized with some template
>> (similar to informational notes):
>>
>> file line:column note:  is ed >
>>   where  is a source construct such as a loop, a branch, a
>> function etc, while  is the transformation such as 'vectorized',
>> 'unrolled', 'peeled', 'if converted', 'hoisted' etc. Additional
>> information can be something to describe more about the transformation
>> and the source construct. For instance, unrolled N times, unrolled
>> completely,   and profile information of the loop (entry count,
>> average trip count etc). The addtitional information needs to be
>> concise. Please do *not* dump with verbosity as you proposed
>> (including the size, induction variable folding, exit condition
>> elimination etc).
>>
>> 4) the existing dump (into the dump file) can be changed to use the
>> same dump format above
>> 5) For loop unroll/peeling, the dumping code can be refactorized using
>> one report function -- see the code in google branch
>>
>> 6) do not forget the tree level unroller.
>>
>> David
>>
>> On Thu, Dec 13, 2012 at 6:15 PM, Sharad Singhai  wrote:
>>> Hi,
>>>
>>> As per discussion in http://gcc.gnu.org/ml/gcc/2012-12/msg00056.html,
>>> the attached patch updates loop unroll passes to use new dump
>>> infrastructure.
>>>
>>> This patch filters relevant dump messages into the following
>>> three categories
>>>
>>> - optimized: an optimization was successfully applied
>>> - missed: an optimization was missed due to the described reason
>>> - note: other relevant/detailed info during optimization. For example,
>>>   loop unrolling computes the loop bounds and size.
>>>
>>> Two sample outputs from one of the gcc tests (gcc.dg/unroll_1.c) are below.
>>>
>>> Sample 1
>>> -- info about optimized loops via
>>> "-fopt-info-loop-optimized" ---
>>> $ gcc gcc.dg/unroll_1.c -fno-diagnostics-show-caret -O2 -S
>>> -fdump-rtl-loop2_unroll -funroll-loops -fopt-info-loop-optimized
>>>
>>> Unrolled loop 1 completely (duplicated 2 times).
>>> Exit condition of peeled iterations was eliminated.
>>> Last iteration exit edge was proved true.
>>> Unrolled loop 1 completely (duplicated 2 times).
>>> Exit condition of peeled iterations was eliminated.
>>> Last iteration exit edge was proved true.
>>> 
>>>
>>> Sample 2:
>>> --- All available loop optimization info, i.e., optimized+missed+note
>>> via "-fopt-info-loop" ---
>>> $ gcc gcc.dg/unroll_1.c -fno-diagnostics-show-caret -O2 -S
>>> -fdump-rtl-loop2_unroll -funroll-loops -fopt-info-loop
>>>
>>> Loop 1 iterates 2 times.
>>> Loop 1 iterates at most 2 times.
>>

[PATCH] Fix ICE in vectorization dump (PR tree-optimization/55995)

2013-01-18 Thread Sharad Singhai
This patch fixes an ICE in vectorization dump when section anchors are present.

Bootstrapped/tested on x86_64 and PPC 64 and found no new failures. OK
for trunk?

Thanks,
Sharad

2013-01-18  Sharad Singhai  

PR tree-optimization/55995
* dumpfile.c (dump_loc): Print location only if available.
* testsuite/gcc.dg/vect/vect.exp: Use "details" flags for dump info.
* tree-vectorizer.c (increase_alignment): Intialize vect_location.

Index: dumpfile.c
===
--- dumpfile.c (revision 195244)
+++ dumpfile.c (working copy)
@@ -260,14 +260,13 @@ dump_loc (int dump_kind, FILE *dfile, source_locat
   /* Currently vectorization passes print location information.  */
   if (dump_kind)
 {
-  if (loc == UNKNOWN_LOCATION)
+  if (loc != UNKNOWN_LOCATION)
+fprintf (dfile, "\n%s:%d: note: ", LOCATION_FILE (loc),
+ LOCATION_LINE (loc));
+  else if (current_function_decl)
 fprintf (dfile, "\n%s:%d: note: ",
  DECL_SOURCE_FILE (current_function_decl),
  DECL_SOURCE_LINE (current_function_decl));
- else
-fprintf (dfile, "\n%s:%d: note: ",
- LOCATION_FILE (loc),
- LOCATION_LINE (loc));
 }
 }

Index: testsuite/gcc.dg/vect/vect.exp
===
--- testsuite/gcc.dg/vect/vect.exp (revision 195244)
+++ testsuite/gcc.dg/vect/vect.exp (working copy)
@@ -156,7 +156,8 @@ dg-runtest [lsort [glob -nocomplain $srcdir/$subdi

 # alignment-sensitive -fsection-anchors tests
 set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS
-lappend DEFAULT_VECTCFLAGS "-fsection-anchors" "-fdump-ipa-increase_alignment"
+lappend DEFAULT_VECTCFLAGS "-fsection-anchors" \
+ "-fdump-ipa-increase_alignment-details"
 dg-runtest [lsort [glob -nocomplain
$srcdir/$subdir/aligned-section-anchors-*.\[cS\]]]  \
  "" $DEFAULT_VECTCFLAGS

Index: tree-vectorizer.c
===
--- tree-vectorizer.c (revision 195244)
+++ tree-vectorizer.c (working copy)
@@ -225,6 +225,8 @@ increase_alignment (void)
 {
   struct varpool_node *vnode;

+  vect_location = UNKNOWN_LOC;
+
   /* Increase the alignment of all global arrays for vectorization.  */
   FOR_EACH_DEFINED_VARIABLE (vnode)
 {


Re: [PATCH] Add an intermediate coverage format for gcov

2011-10-18 Thread Sharad Singhai
On Wed, Oct 5, 2011 at 9:58 AM, Mike Stump  wrote:
>
> On Oct 5, 2011, at 12:47 AM, Sharad Singhai wrote:
> > This patch adds an intermediate coverage format (enabled via 'gcov
> > -i'). This is a compact format as it does not require source files.
>
> I don't like any of the tags, I think if you showed them to 100 people that 
> had used gcov before, very few of them would be able to figure out the 
> meaning without reading the manual.  Why make it completely cryptic?  A 
> binary encoded compress format is smaller and just as readable.
>
> SF, sounds like single float to me, I'd propose file.
> FN, sounds like filename, I'd propose line.
> FNDA, can't even guess at what it means, I'd propose fcount.
> BA, I'd propose branch, for 0, notexe, for 1, nottaken, for 2 taken.
> DA, I'd propose lcount
>
> I'd propose fcount be listed as fname,ecount, to match the ordering of 
> lcount.  Also, implicit in the name, is the ordering f, then count, l, then 
> count.
>
> I think if you showed the above to 100 people that had used gcov before, I 
> think we'd be up past 90% of the people able to figure it out, without 
> reading the doc.

Okay, I liked the idea of self-descriptive tags. I have updated the
patch based on your suggestions. I have simplified the format
somewhat. Instead of repeating function name, I use a 'function' tag
with the format

function:,,

I also dropped the unmangled function names, they were turning out to
be too unreadable and not really useful in this context.

Here is the updated patch. OK for trunk?

2011-10-18   Sharad Singhai  

* doc/gcov.texi: Document gcov intermediate format.
* gcov.c (print_usage): Handle new option.
(process_args): Handle new option.
(get_gcov_file_intermediate_name): New function.
(output_intermediate_file): New function.
(generate_results): Handle new option.
* testsuite/lib/gcov.exp: Handle intermediate format.
* testsuite/g++.dg/gcov/gcov-8.C: New testcase.

Index: doc/gcov.texi
===
--- doc/gcov.texi   (revision 179873)
+++ doc/gcov.texi   (working copy)
@@ -130,6 +130,7 @@ gcov [@option{-v}|@option{--version}] [@
  [@option{-f}|@option{--function-summaries}]
  [@option{-o}|@option{--object-directory} @var{directory|file}]
@var{sourcefiles}
  [@option{-u}|@option{--unconditional-branches}]
+ [@option{-i}|@option{--intermediate-format}]
  [@option{-d}|@option{--display-progress}]
 @c man end
 @c man begin SEEALSO
@@ -216,6 +217,45 @@ Unconditional branches are normally not
 @itemx --display-progress
 Display the progress on the standard output.

+@item -i
+@itemx --intermediate-format
+Output gcov file in an easy-to-parse intermediate text format that can
+be used by @command{lcov} or other tools. The output is a single
+@file{.gcov} file per @file{.gcda} file. No source code is required.
+
+The format of the intermediate @file{.gcov} file is plain text with
+one entry per line
+
+@smallexample
+file:@var{source_file_name}
+function:@var{function_name},@var{line_number},@var{execution_count}
+lcount:@var{line number},@var{execution_count}
+branch:@var{line_number},@var{branch_coverage_type}
+
+Where the @var{branch_coverage_type} is
+   notexec (Branch not executed)
+   taken (Branch executed and taken)
+   nottaken (Branch executed, but not taken)
+
+There can be multiple @var{file} entries in an intermediate gcov
+file. All entries following a @var{file} pertain to that source file
+until the next @var{file} entry.
+@end smallexample
+
+Here is a sample when @option{-i} is used in conjuction with
@option{-b} option:
+
+@smallexample
+file:array.cc
+function:_Z3sumRKSt6vectorIPiSaIS0_EE,11,1
+function:main,22,1
+lcount:11,1
+lcount:12,1
+lcount:14,1
+branch:14,taken
+lcount:26,1
+branch:28,nottaken
+@end smallexample
+
 @end table

 @command{gcov} should be run with the current directory the same as that
Index: gcov.c
===
--- gcov.c  (revision 179873)
+++ gcov.c  (working copy)
@@ -311,6 +311,9 @@ static int flag_gcov_file = 1;

 static int flag_display_progress = 0;

+/* Output *.gcov file in intermediate format used by 'lcov'.  */
+static int flag_intermediate_format = 0;
+
 /* For included files, make the gcov output file name include the name
of the input source file.  For example, if x.h is included in a.c,
then the output file name is a.c##x.h.gcov instead of x.h.gcov.  */
@@ -436,6 +439,7 @@ print_usage (int error_p)
   fnotice (file, "  -o, --object-directory DIR|FILE Search for object
files in DIR or called FILE\n");
   fnotice (file, "  -p, --preserve-pathsPreserve all
pathname components\n");
   fnotice (file, "  -u, --unconditional

Re: [PATCH] Add an intermediate coverage format for gcov

2011-10-19 Thread Sharad Singhai
Since the updated patch already uses unmangled function names, is it
good to commit then?

Sharad

On Wed, Oct 19, 2011 at 1:48 AM, Jan Hubicka  wrote:
>> On Oct 18, 2011, at 4:19 PM, Sharad Singhai  wrote:
>> > Okay, I liked the idea of self-descriptive tags. I have updated the
>> > patch based on your suggestions. I have simplified the format
>> > somewhat. Instead of repeating function name, I use a 'function' tag
>> > with the format
>> >
>> > function:,,
>>
>> Sound nice.
>>
>> > I also dropped the unmangled function names, they were turning out to
>> > be too unreadable and not really useful in this context.
>>
>> Ah, I'd argue for mangled names.  Every one knows they can stream through 
>> c++filt and get unmangled, but once you unmangle, you can never go back.  
>> Also, the mangled version is significantly smaller.  For c, it is 
>> irrelevant, for c++, it makes a big difference.
>
> I would also support unmangled variant. Otherwise the patch seems resonable 
> to me.
>
> Honza
>> >
>


Re: [PATCH] Add an intermediate coverage format for gcov

2011-10-19 Thread Sharad Singhai
Sorry, I misunderstood your comment. I see that you are asking for
unmangled function names whereas the current patch supports only
mangled names. I can print unmangled names under another option. Would
that work?

Thanks,
Sharad

On Wed, Oct 19, 2011 at 12:06 PM, Sharad Singhai  wrote:
> Since the updated patch already uses unmangled function names, is it
> good to commit then?
>
> Sharad
>
> On Wed, Oct 19, 2011 at 1:48 AM, Jan Hubicka  wrote:
>>> On Oct 18, 2011, at 4:19 PM, Sharad Singhai  wrote:
>>> > Okay, I liked the idea of self-descriptive tags. I have updated the
>>> > patch based on your suggestions. I have simplified the format
>>> > somewhat. Instead of repeating function name, I use a 'function' tag
>>> > with the format
>>> >
>>> > function:,,
>>>
>>> Sound nice.
>>>
>>> > I also dropped the unmangled function names, they were turning out to
>>> > be too unreadable and not really useful in this context.
>>>
>>> Ah, I'd argue for mangled names.  Every one knows they can stream through 
>>> c++filt and get unmangled, but once you unmangle, you can never go back.  
>>> Also, the mangled version is significantly smaller.  For c, it is 
>>> irrelevant, for c++, it makes a big difference.
>>
>> I would also support unmangled variant. Otherwise the patch seems resonable 
>> to me.
>>
>> Honza
>>> >
>>
>


Re: [PATCH] Add an intermediate coverage format for gcov

2011-11-06 Thread Sharad Singhai
Sorry about the delay. I have updated the patch to output demangled
names under a new option (-m) and added a test case. Okay for trunk?

Sharad

2011-11-06   Sharad Singhai  

* doc/gcov.texi: Document gcov intermediate format.
* gcov.c (print_usage): Handle new option.
(process_args): Handle new option.
(get_gcov_file_intermediate_name): New function.
(output_intermediate_file): New function.
(generate_results): Handle new option.
* testsuite/lib/gcov.exp: Handle intermediate format.
* testsuite/g++.dg/gcov/gcov-8.C: New testcase.
* testsuite/g++.dg/gcov/gcov-9.C: New testcase.

Index: doc/gcov.texi
===
--- doc/gcov.texi   (revision 179873)
+++ doc/gcov.texi   (working copy)
@@ -131,6 +131,8 @@ gcov [@option{-v}|@option{--version}] [@
  [@option{-o}|@option{--object-directory} @var{directory|file}]
@var{sourcefiles}
  [@option{-u}|@option{--unconditional-branches}]
  [@option{-d}|@option{--display-progress}]
+ [@option{-i}|@option{--intermediate-format}]
+ [@option{-m}|@option{--demangled-names}]
 @c man end
 @c man begin SEEALSO
 gpl(7), gfdl(7), fsf-funding(7), gcc(1) and the Info entry for @file{gcc}.
@@ -216,6 +218,49 @@ Unconditional branches are normally not
 @itemx --display-progress
 Display the progress on the standard output.

+@item -i
+@itemx --intermediate-format
+Output gcov file in an easy-to-parse intermediate text format that can
+be used by @command{lcov} or other tools. The output is a single
+@file{.gcov} file per @file{.gcda} file. No source code is required.
+
+The format of the intermediate @file{.gcov} file is plain text with
+one entry per line
+
+@smallexample
+file:@var{source_file_name}
+function:@var{line_number},@var{execution_count},@var{function_name}
+lcount:@var{line number},@var{execution_count}
+branch:@var{line_number},@var{branch_coverage_type}
+
+Where the @var{branch_coverage_type} is
+   notexec (Branch not executed)
+   taken (Branch executed and taken)
+   nottaken (Branch executed, but not taken)
+
+There can be multiple @var{file} entries in an intermediate gcov
+file. All entries following a @var{file} pertain to that source file
+until the next @var{file} entry.
+@end smallexample
+
+Here is a sample when @option{-i} is used in conjuction with
@option{-b} option:
+
+@smallexample
+file:array.cc
+function:11,1,_Z3sumRKSt6vectorIPiSaIS0_EE
+function:22,1,main
+lcount:11,1
+lcount:12,1
+lcount:14,1
+branch:14,taken
+lcount:26,1
+branch:28,nottaken
+@end smallexample
+
+@item -m
+@itemx --demangled-names
+Display demangled function names in output.
+
 @end table

 @command{gcov} should be run with the current directory the same as that
Index: gcov.c
===
--- gcov.c  (revision 179873)
+++ gcov.c  (working copy)
@@ -39,6 +39,7 @@ along with Gcov; see the file COPYING3.
 #include "intl.h"
 #include "diagnostic.h"
 #include "version.h"
+#include "demangle.h"

 #include 

@@ -168,6 +169,7 @@ typedef struct function_info
 {
   /* Name of function.  */
   char *name;
+  char *demangled_name;
   unsigned ident;
   unsigned lineno_checksum;
   unsigned cfg_checksum;
@@ -311,6 +313,14 @@ static int flag_gcov_file = 1;

 static int flag_display_progress = 0;

+/* Output *.gcov file in intermediate format used by 'lcov'.  */
+
+static int flag_intermediate_format = 0;
+
+/* Output demangled function names.  */
+
+static int flag_demangled_names = 0;
+
 /* For included files, make the gcov output file name include the name
of the input source file.  For example, if x.h is included in a.c,
then the output file name is a.c##x.h.gcov instead of x.h.gcov.  */
@@ -433,9 +443,11 @@ print_usage (int error_p)
   fnotice (file, "  -l, --long-file-names   Use long output
file names for included\n\
 source files\n");
   fnotice (file, "  -f, --function-summariesOutput summaries
for each function\n");
+  fnotice (file, "  -m, --demangled-names   Output demangled
function names\n");
   fnotice (file, "  -o, --object-directory DIR|FILE Search for object
files in DIR or called FILE\n");
   fnotice (file, "  -p, --preserve-pathsPreserve all
pathname components\n");
   fnotice (file, "  -u, --unconditional-branchesShow
unconditional branch counts too\n");
+  fnotice (file, "  -i, --intermediate-format   Output .gcov file
in intermediate text format\n");
   fnotice (file, "  -d, --display-progress  Display progress
information\n");
   fnotice (file, "\nFor bug reporting instructions, please see:\n%s.\n",
   bug_report_url);
@@ -467,11 +479,13 @@ static const struct option options[] =
   { "no-output",   

Re: Disable early inlining while compiling for coverage (issue5173042)

2011-11-07 Thread Sharad Singhai
Honza,
Sorry, I forgot about this. Could you please put this on your TODO list?

David,
While a proper fix is pending for the trunk, we need this interim fix
internally. Okay for google/main?

Thanks,
Sharad

On Sun, Oct 2, 2011 at 3:08 AM, Jan Hubicka  wrote:
>>
>> I believe Richi opent PR back when he introduced the SSA profiling, but I 
>> don;t seem
>> to be able to find it now.
> Ha, you found it. It is PR gcov-profile/45890.
>
> Given outcome of this dicussion I think it would make most sense to make 
> coverage just after into-SSA
> (and perhaps arrange cfgcleanups run before to be sensitive WRT line number 
> infos).
> Path is welcome, otherwise I will try to put this on my TODO for 4.7
>
> Thanks!
> Honza
>>
>> Honza
>


Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-10 Thread Sharad Singhai
Ping.

Thanks,
Sharad
Sharad


On Wed, Sep 5, 2012 at 10:34 AM, Sharad Singhai  wrote:
> Ping.
>
> Thanks,
> Sharad
>
> Sharad
>
>
>
>
> On Fri, Aug 24, 2012 at 1:06 AM, Sharad Singhai  wrote:
>>
>> Sorry about the delay. Please see comments inline.
>>
>> On Wed, Jul 4, 2012 at 6:33 AM, Richard Guenther
>>  wrote:
>> > On Tue, Jul 3, 2012 at 11:07 PM, Sharad Singhai 
>> > wrote:
>> >> Apologies for the spam. Attempting to resend the patch after shrinking
>> >> it.
>> >>
>> >> I have updated the attached patch to use a new dump message
>> >> classification system for the vectorizer. It currently uses four
>> >> classes, viz, MSG_OPTIMIZED_LOCATIONS, MSG_UNOPTIMIZED_LOCATION,
>> >> MSG_MISSING_OPTIMIZATION, and MSG_NOTE. I have gone through the
>> >> vectorizer passes and have converted each call to fprintf (dump_file,
>> >> ) to a message classification matching in spirit. Most often, it
>> >> is MSG_OPTIMIZED_LOCATIONS, but occasionally others as well.
>> >>
>> >> For example, the following
>> >>
>> >> if (vect_print_dump_info (REPORT_DETAILS))
>> >>   {
>> >> fprintf (vect_dump, "niters for prolog loop: ");
>> >> print_generic_expr (vect_dump, iters, TDF_SLIM);
>> >>   }
>> >>
>> >> gets converted to
>> >>
>> >> if (dump_kind (MSG_OPTIMIZED_LOCATIONS))
>> >>   {
>> >>  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
>> >>   "niters for prolog loop: ");
>> >>  dump_generic_expr (MSG_OPTIMIZED_LOCATIONS, TDF_SLIM, iters);
>> >>   }
>> >>
>> >> The asymmetry between the first printf and the second is due to the
>> >> fact that 'vect_print_dump_info (xxx)' prints the location as a
>> >> "side-effect". To preserve the original intent somewhat, I have
>> >> converted the first call within a dump sequence to a dump_printf_loc
>> >> (xxx) which prints the location while the subsequence calls within the
>> >> same conditional get converted to the corresponding plain variants.
>> >
>> > Ok, that looks reasonable.
>> >
>> >> I considered removing the support for alternate dump file, but ended
>> >> up preserving it instead since it is needed for setting the alternate
>> >> dump file to stderr for the case when -fopt-info is given but no dump
>> >> file is available.
>> >>
>> >> The following invocation
>> >> g++ ... -ftree-vectorize -fopt-info=4
>> >>
>> >> dumps *all* available information to stderr. Currently, the opt-info
>> >> level is common to all passes, i.e., a pass can't specify if wants a
>> >> different level of diagnostic info. This can be added as an
>> >> enhancement with a suitable syntax for selecting passes.
>> >>
>> >> I haven't fixed up the documentation/tests but wanted to get some
>> >> feedback about the current state of patch before doing that.
>> >
>> > Some comments / questions.
>> >
>> > +  if (dump_file && (dump_kind & opt_info_flags))
>> > +{
>> > +  dump_loc (dump_kind, dump_file, loc);
>> > +  print_generic_expr (dump_file, t, dump_flags | extra_dump_flags);
>> > +}
>> > +
>> > +  if (alt_dump_file && (dump_kind & opt_info_flags))
>> > +{
>> >
>> > you always test dump_kind against the same opt_info_flags variable.
>> > I would have thought that the alternate dump file has a different
>> > opt_info_flags
>> > setting so I can have -fdump-tree-vect-details -fopt-info=1.  Am I
>> > missing
>> > something?
>>
>> It was an oversight on my part. I have since fixed this. There are two
>> separate flags corresponding to the two types of dump files,
>>
>> pflags ==> pass private dump file
>> alt_flags ==> opt-info dump file
>>
>> > If I do
>> >
>> >> gcc file1.c file2.c -O3 -fdump-tree-vectorize=foo
>> >
>> > what will foo contain afterwards?  I think you need to document the
>> > behavior
>> > when such redirection is used with the compiler-driver feature of
>> > handling
>> > multiple translation units.  Especially the difference (or not
>> > difference) to
>> >

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-13 Thread Sharad Singhai
That is a good point. Currently I am making a distinction between dump
flags and opt-info flags, but it is not necessary since the opt-info
flags can be thought of an extension of dump flags.

I will update the patch so that -fdump-tree-vect-optimized also works.

Thanks,
Sharad

On Thu, Sep 13, 2012 at 4:08 AM, Richard Guenther
 wrote:
> On Wed, Sep 12, 2012 at 6:46 PM, Xinliang David Li  wrote:
>> On Wed, Sep 12, 2012 at 3:30 AM, Richard Guenther
>>  wrote:
>>> On Wed, Sep 12, 2012 at 10:12 AM, Sharad Singhai  wrote:
>>>> Thanks for your comments. Please see my responses inline.
>>>>
>>>> On Tue, Sep 11, 2012 at 1:16 PM, Xinliang David Li  
>>>> wrote:
>>>>> Can you resend your patch in text form (also need to resolve the
>>>>> latest conflicts) so that it can be commented inline?
>>>>
>>>> I tried to include inline patch earlier but my message was bounced
>>>> back from patches mailing list. I am trying it again.
>>>>
>>>>> Please also provide as summary a more up-to-date description of
>>>>> 1) Command line option syntax and semantics
>>>>
>>>> I added some documentation in the patch. Here are the relevant bits
>>>> from invoke.texi.
>>>>
>>>> `-fdump-tree-SWITCH-OPTIONS=FILENAME'
>>>>  Control the dumping at various stages of processing the
>>>>  intermediate language tree to a file.  The file name is generated
>>>>  by appending a switch-specific suffix to the source file name, and
>>>>  the file is created in the same directory as the output file. In
>>>>  case of `=FILENAME' option, the dump is output on the given file
>>>>  instead of the auto named dump files.
>>>>  ...
>>>>
>>>> `=FILENAME'
>>>>   Instead of an auto named dump file, output into the given file
>>>>   name. The file names `stdout' and `stderr' are treated
>>>>   specially and are considered already open standard streams.
>>>>   For example,
>>>>
>>>>gcc -O2 -ftree-vectorize -fdump-tree-vect-details=foo.dump
>>>> -fdump-tree-pre=stderr file.c
>>>>
>>>>   outputs vectorizer dump into `foo.dump', while the PRE dump
>>>>   is output on to `stderr'. If two conflicting dump filenames
>>>>   are given for the same pass, then the latter option
>>>>   overrides the earlier one.
>>>>
>>>> `-fopt-info-PASS'
>>>> `-fopt-info-PASS-OPTIONS'
>>>> `-fopt-info-PASS-OPTIONS=FILENAME'
>>>>  Controls optimization dumps from various passes. If the `-OPTIONS'
>>>>  form is used, OPTIONS is a list of `-' separated options which
>>>>  controls the details of the dump.  If OPTIONS is not specified, it
>>>>  defaults to `optimized'. If the FILENAME is not specified, it
>>>>  defaults to `stderr'. Note that the output FILENAME will be
>>>>  overwritten in case of multiple translation units. If a combined
>>>>  output from multiple the translation units is desired, `stderr'
>>>>  should be used instead.
>>>>
>>>>  The PASS could be one of the tree or rtl passes. The following
>>>>  options are available
>>>
>>> I don't like that we have -PASS here.  That makes it awfully similar
>>> to -fdump-PASS-OPTIONS=FILENAME.  Are we merely having
>>> -fopt-info because OPTIONS are "different"?
>>
>>
>> Having PASS is useful to do filtering. But as your said, the option
>> design here is very much oriented towards developers not end users
>> which fopt-info is also intended for.
>
> Just to add a comment here - -fopt-info is _only_ targeted at end users.
> Developers can use -fdump-tree-XXX=stderr now (which, with the correct
> pass / flags should produce identical output to -fopt-info - at least that
> was the whole point with the re-design of the dump API - to make it
> possible to implement -fopt-info in a way that it simply provides a nice
> interface to end-users to our existing dumping information.
>
> If it doesn't work like that right now we should make it work this way.
>
> Richard.


Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-13 Thread Sharad Singhai
> Is -fopt-info-rtl-all also accepted?

Currently it is accepted. However, based on the recent comments, I am
going to remove the pass name from the flags.

>
> It would be useful to have a good default for -fopt-info so that users
> can get high level info about optimizations without having to specify
> a pass. Can -fopt-info be mapped to
> "-fopt-info-tree-all-optimized=stderr

Yes, I have updated the patch so that -fopt-info without any pass
specifier would map to all passes which support this. Currently, only
vectorizer.

> -fopt-info-rtl-all-optimized=stderr"? And perhaps
> -fopt-info-all-OPTIONS can be mapped to all tree and rtl passes?
>
> Thanks,
> Teresa

Thanks,
Sharad


Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-18 Thread Sharad Singhai
On Sep 18, 2012 8:43 AM, "Xinliang David Li"  wrote:
>
> On Tue, Sep 18, 2012 at 1:48 AM, Sharad Singhai  wrote:
> > In response to the recent comments, I have updated the patch to do the
> > following:
> >
> > - Remove pass handling from -fopt-info
> > - Support additional flags in regular dumps
> >
> > I have massaged the options so that they have the following (hopefully
> > clearer) behavior:
> >
> > gcc ... -fopt-info    ---> dump all optimization info on stderr
> > gcc ... -fopt-info-missed-optimized=file.txt  --> dump info about
> > optimization applied as well as missed opportunities on to file.txt.
> > If no file.txt is provided, then use stderr.
> >
> > I have enhanced regular dump flags, so that values accepted by
> > -fopt-info are also accepted. For example,
> > gcc ... -O2 -ftree-vectorize -fdump-tree-vect-optimized=foo.dump
> >
> > Now foo.dump will include the regular tree-vect dump as well as the
> > output of -fopt-info=optimized. This way developers can get more
> > detailed dumps when needed.
> >
> > I have also changed the meaning of dump option "details" to include
> > optimization details. Thus "-details" flag implies
> > "-missed-optimized-note" in addition to other dumps.
> >
> > The pass level filtering of -fopt-info dumps can be done in a follow
> > up patch. It may even turn out to be unnecessary, because the
> > equivalent effect can be achieved by
> > -ftree-PASS-optimized-missed-note.
> >
>
> Richard's suggestion to map high level 'pass' names to internal passes
> and make it available to -fopt-info filtering for end users as a
> follow up pass will be useful.


Yes, certainly. I plan to do that in a follow up patch. Currently only
vectorization passes use the new dump infrastructure. But as more
passes get converted, it will be nice to have an option for high-level
-fopt-info filtering for end users.

I presume a group of passes would be covered under a single -fopt-info
name, such as loop-optimizations. The exact scheme is yet to be
designed/discussed.

Thanks,
Sharad

>
>
> thanks,
>
> David
>
>
>
> > I have bootstrapped and tested the attached patch on x86_64 and didn't
> > observe any new failures. Okay for trunk?
> >
> > Thanks,
> > Sharad


Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-24 Thread Sharad Singhai
Ping.


Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-09-27 Thread Sharad Singhai
Thanks for the review. A couple of comments inline:

> Some minor issues:
>
> * c/c-decl.c (c_write_global_declarations): Use different method to
> determine if the dump has ben initialized.
> * cp/decl2.c (cp_write_global_declarations): Ditto.
> * testsuite/gcc.target/i386/vect-double-1.c: Fix test.
>
> these subdirs all have their separate ChangeLog entry from where the
> directory name is omitted.
>
> Index: tree-dump.c
> ===
> --- tree-dump.c (revision 191490)
> +++ tree-dump.c (working copy)
> @@ -24,9 +24,11 @@ along with GCC; see the file COPYING3.  If not see
>  #include "coretypes.h"
>  #include "tm.h"
>  #include "tree.h"
> +#include "gimple-pretty-print.h"
>  #include "splay-tree.h"
>  #include "filenames.h"
>  #include "diagnostic-core.h"
> +#include "rtl.h"
>
> what do you need gimple-pretty-print.h and rtl.h for?
>
> +
> +extern void dump_bb (FILE *, basic_block, int, int);
> +
>
> that should be declared in some header
>
> +/* Dump gimple statement GS with SPC indentation spaces and
> +   EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is enabled.  */
> +
> +void
> +dump_gimple_stmt (int dump_kind, int extra_dump_flags, gimple gs, int spc)
> +{
>
> the gimple stuff really belongs in to gimple-pretty-print.c

This dump_gimple_stmt () is just a dispatcher, which uses internal
data structure such as dump streams/flags. If I move it into
gimple-pretty-print.c, then I would have to export those
streams/flags. I was hoping to avoid it by keeping all dump_* ()
methods together in dumpfile.c (earlier in tree-dump.c). Thus, later
one could just make dump_file/dump_flags static when all the passes
have converted to this scheme.

>
> (parts of tree-dump.c should be moved to a new file dumpfile.c)
>
> +/* Dump tree T using EXTRA_DUMP_FLAGS on dump streams if DUMP_KIND is
> +   enabled.  */
> +
> +void
> +dump_generic_expr (int dump_kind, int extra_dump_flags, tree t)
> +{
>
> belongs to tree-pretty-print.c (to where the routines are it calls)

This is again a dispatcher for dump_generic_expr () which writes to
the appropriate stream depending upon dump_kind.

>
> +int
> +dump_start (int phase, int *flag_ptr)
> +{
>
> perfect candidate for dumpfile.c
>
> You can do this re-shuffling as followup, but please try to not include rtl.h
> or gimple-pretty-print.h from tree-dump.c.  Thus re-shuffling required by that
> do now.  tree-dump.c should only know about dumping 'tree'.

Okay, I have moved relevant methods into dumpfile.c.

>
> Index: tree-dump.h
> ===
> --- tree-dump.h (revision 191490)
> +++ tree-dump.h (working copy)
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
>  #ifndef GCC_TREE_DUMP_H
>  #define GCC_TREE_DUMP_H
>
> +#include "input.h"
>
> probably no longer required.
>
> Index: dumpfile.h
> ===
> --- dumpfile.h  (revision 191490)
> +++ dumpfile.h  (working copy)
> @@ -22,6 +22,9 @@ along with GCC; see the file COPYING3.  If not see
>  #ifndef GCC_DUMPFILE_H
>  #define GCC_DUMPFILE_H 1
>
> +#include "coretypes.h"
> +#include "input.h"
>
> likewise for input.h.
>
> Index: testsuite/gcc.target/i386/vect-double-1.c
> ===
> --- testsuite/gcc.target/i386/vect-double-1.c   (revision 191490)
> +++ testsuite/gcc.target/i386/vect-double-1.c   (working copy)
> @@ -32,5 +32,5 @@ sse2_test (void)
>  }
>  }
>
> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
> +/* { dg-final { scan-tree-dump-times "Vectorized loops: 1" 1 "vect" } } */
>  /* { dg-final { cleanup-tree-dump "vect" } } */
>
> I am sure you need a gazillion more testsuite adjustments?  Thus, did you
> really test the patch by a bootstrap and a toplevel make -k check for
> regressions?
>
> Index: opts.c
> ===
> --- opts.c  (revision 191490)
> +++ opts.c  (working copy)
> @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "system.h"
>  #include "intl.h"
>  #include "coretypes.h"
> +#include "dumpfile.h"
>
> I don't see that you add a use for this.  Please double-check all your include
> file changes.
>
> Index: gimple-pretty-print.c
> ===
> --- gimple-pretty-print.c   (revision 191490)
> +++ gimple-pretty-print.c   (working copy)
> @@ -69,7 +69,7 @@ maybe_init_pretty_print (FILE *file)
>  }
> ...
> Index: gimple-pretty-print.h
> ===
> --- gimple-pretty-print.h   (revision 191490)
> +++ gimple-pretty-print.h   (working copy)
> @@ -31,6 +31,6 @@ extern void debug_gimple_seq (gimple_seq);
>  extern void print_gimple_seq (FILE *, gimple_seq, int, int);
>  extern void print_

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-06-13 Thread Sharad Singhai
Thanks for your comments. Responses inline.

On Wed, Jun 13, 2012 at 4:48 AM, Richard Guenther
 wrote:
> On Fri, Jun 8, 2012 at 7:16 AM, Sharad Singhai  wrote:
>> Okay, I have updated the attached patch so that the output from
>> -ftree-vectorizer-verbose is considered diagnostic information and is
>> always
>> sent to stderr. Other functionality remains unchanged. Here is some
>> more context about this patch.
>>
>> This patch improves the dump infrastructure and public interfaces so
>> that the existing private pass-specific dump stream is separated from
>> the diagnostic dump stream (typically stderr).  The optimization
>> passes can output information on the two streams independently.
>>
>> The newly defined interfaces are:
>>
>> Individual passes do not need to access the dump file directly. Thus Instead
>> of doing
>>
>>   if (dump_file && (flags & dump_flags))
>>      fprintf (dump_file, ...);
>>
>> they can do
>>
>>     dump_printf (flags, ...);
>>
>> If the current pass has FLAGS enabled then the information gets
>> printed into the dump file otherwise not.
>>
>> Similar to the dump_printf (), another function is defined, called
>>
>>        diag_printf (dump_flags, ...)
>>
>> This prints information only onto the diagnostic stream, typically
>> standard error. It is useful for separating pass-specific dump
>> information from
>> the diagnostic information.
>>
>> Currently, as a proof of concept, I have converted vectorizer passes
>> to use the new dump format. For this, I have considered
>> information printed in vect_dump file as diagnostic. Thus 'fprintf'
>> calls are changed to 'diag_printf'. Some other information printed to
>> dump_file is sent to the regular dump file via 'dump_printf ()'. It
>> helps to separate the two streams because they might serve different
>> purposes and might have different formatting requirements.
>>
>> For example, using the trunk compiler, the following invocation
>>
>> g++ -S v.cc -ftree-vectorize -fdump-tree-vect -ftree-vectorizer-verbose=2
>>
>> prints tree vectorizer dump into a file named 'v.cc.113t.vect'.
>> However, the verbose diagnostic output is silently
>> ignored. This is not desirable as the two types of dump should not interfere.
>>
>> After this patch, the vectorizer dump is available in 'v.cc.113t.vect'
>> as before, but the verbose vectorizer diagnostic is additionally
>> printed on stderr. Thus both types of dump information are output.
>>
>> An additional feature of this patch is that individual passes can
>> print dump information into command-line named files instead of auto
>> numbered filename. For example,
>
> I'd wish you'd leave out this part for a followup.

I thought you wanted all parts together. Anyway, I can remove this part.

>
>>
>> g++ -S -O2 v.cc -ftree-vectorize -fdump-tree-vect=foo.vect
>>     -ftree-vectorizer-verbose=2
>>     -fdump-tree-pre=foo.pre
>>
>> This prints the tree vectorizer dump into 'foo.vect', PRE dump into
>> 'foo.pre', and the vectorizer verbose diagnostic dump onto stderr.
>>
>> Please take another look.
>
> --- tree-vect-loop-manip.c      (revision 188325)
> +++ tree-vect-loop-manip.c      (working copy)
> @@ -789,14 +789,11 @@ slpeel_make_loop_iterate_ntimes (struct loop *loop
>   gsi_remove (&loop_cond_gsi, true);
>
>   loop_loc = find_loop_location (loop);
> -  if (dump_file && (dump_flags & TDF_DETAILS))
> -    {
> -      if (loop_loc != UNKNOWN_LOC)
> -        fprintf (dump_file, "\nloop at %s:%d: ",
> +  if (loop_loc != UNKNOWN_LOC)
> +    dump_printf (TDF_DETAILS, "\nloop at %s:%d: ",
>                  LOC_FILE (loop_loc), LOC_LINE (loop_loc));
> -      print_gimple_stmt (dump_file, cond_stmt, 0, TDF_SLIM);
> -    }
> -
> +  if (dump_flags & TDF_DETAILS)
> +    dump_gimple_stmt (TDF_SLIM, cond_stmt, 0);
>   loop->nb_iterations = niters;
>
> I'm confused by this.  Why is this not simply
>
>  if (loop_loc != UNKNOWN_LOC)
>    dump_printf (dump_flags, "\nloop at %s:%d: ",
>                       LOC_FILE (loop_loc), LOC_LINE (loop_loc));
>  dump_gimple_stmt (dump_flags | TDF_SLIM, cond_stmt, 0);
>
> for example.  I notice that you maybe mis-understood the message 
> classification
> I asked you to add (maybe I confused you by mentioning to eventually re-use
> the TDF_* flags).  I think you basically provided this message classification
> by adding

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-06-14 Thread Sharad Singhai
On Wed, Jun 13, 2012 at 4:48 AM, Richard Guenther
 wrote:
> On Fri, Jun 8, 2012 at 7:16 AM, Sharad Singhai  wrote:
>> Okay, I have updated the attached patch so that the output from
>> -ftree-vectorizer-verbose is considered diagnostic information and is
>> always
>> sent to stderr. Other functionality remains unchanged. Here is some
>> more context about this patch.
>>
>> This patch improves the dump infrastructure and public interfaces so
>> that the existing private pass-specific dump stream is separated from
>> the diagnostic dump stream (typically stderr).  The optimization
>> passes can output information on the two streams independently.
>>
>> The newly defined interfaces are:
>>
>> Individual passes do not need to access the dump file directly. Thus Instead
>> of doing
>>
>>   if (dump_file && (flags & dump_flags))
>>      fprintf (dump_file, ...);
>>
>> they can do
>>
>>     dump_printf (flags, ...);
>>
>> If the current pass has FLAGS enabled then the information gets
>> printed into the dump file otherwise not.
>>
>> Similar to the dump_printf (), another function is defined, called
>>
>>        diag_printf (dump_flags, ...)
>>
>> This prints information only onto the diagnostic stream, typically
>> standard error. It is useful for separating pass-specific dump
>> information from
>> the diagnostic information.
>>
>> Currently, as a proof of concept, I have converted vectorizer passes
>> to use the new dump format. For this, I have considered
>> information printed in vect_dump file as diagnostic. Thus 'fprintf'
>> calls are changed to 'diag_printf'. Some other information printed to
>> dump_file is sent to the regular dump file via 'dump_printf ()'. It
>> helps to separate the two streams because they might serve different
>> purposes and might have different formatting requirements.
>>
>> For example, using the trunk compiler, the following invocation
>>
>> g++ -S v.cc -ftree-vectorize -fdump-tree-vect -ftree-vectorizer-verbose=2
>>
>> prints tree vectorizer dump into a file named 'v.cc.113t.vect'.
>> However, the verbose diagnostic output is silently
>> ignored. This is not desirable as the two types of dump should not interfere.
>>
>> After this patch, the vectorizer dump is available in 'v.cc.113t.vect'
>> as before, but the verbose vectorizer diagnostic is additionally
>> printed on stderr. Thus both types of dump information are output.
>>
>> An additional feature of this patch is that individual passes can
>> print dump information into command-line named files instead of auto
>> numbered filename. For example,
>
> I'd wish you'd leave out this part for a followup.
>
>>
>> g++ -S -O2 v.cc -ftree-vectorize -fdump-tree-vect=foo.vect
>>     -ftree-vectorizer-verbose=2
>>     -fdump-tree-pre=foo.pre
>>
>> This prints the tree vectorizer dump into 'foo.vect', PRE dump into
>> 'foo.pre', and the vectorizer verbose diagnostic dump onto stderr.
>>
>> Please take another look.
>
> --- tree-vect-loop-manip.c      (revision 188325)
> +++ tree-vect-loop-manip.c      (working copy)
> @@ -789,14 +789,11 @@ slpeel_make_loop_iterate_ntimes (struct loop *loop
>   gsi_remove (&loop_cond_gsi, true);
>
>   loop_loc = find_loop_location (loop);
> -  if (dump_file && (dump_flags & TDF_DETAILS))
> -    {
> -      if (loop_loc != UNKNOWN_LOC)
> -        fprintf (dump_file, "\nloop at %s:%d: ",
> +  if (loop_loc != UNKNOWN_LOC)
> +    dump_printf (TDF_DETAILS, "\nloop at %s:%d: ",
>                  LOC_FILE (loop_loc), LOC_LINE (loop_loc));
> -      print_gimple_stmt (dump_file, cond_stmt, 0, TDF_SLIM);
> -    }
> -
> +  if (dump_flags & TDF_DETAILS)
> +    dump_gimple_stmt (TDF_SLIM, cond_stmt, 0);
>   loop->nb_iterations = niters;
>
> I'm confused by this.  Why is this not simply
>
>  if (loop_loc != UNKNOWN_LOC)
>    dump_printf (dump_flags, "\nloop at %s:%d: ",
>                       LOC_FILE (loop_loc), LOC_LINE (loop_loc));
>  dump_gimple_stmt (dump_flags | TDF_SLIM, cond_stmt, 0);
>
> for example.  I notice that you maybe mis-understood the message 
> classification
> I asked you to add (maybe I confused you by mentioning to eventually re-use
> the TDF_* flags).  I think you basically provided this message classification
> by adding two classes by providing both dump_gimple_stmt and diag_gimple_stmt.
> But still in the above you keep a dump_flags t

[PATCH] Add option for dumping to stderr (issue6190057)

2012-05-07 Thread Sharad Singhai
This is the first patch for planned improvements to dump
infrastructure.  Please reference the discussion in
http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02088.html.

The following small patch allows selective tree and rtl dumps on
stderr instead of named files.  Later -fopt-info can be implemented in
form of -fdump-xxx-stderr.

Bootstrapped and tested on x86_64 with one added testcase.  Okay for
trunk?

Thanks,
Sharad

2012-05-07   Sharad Singhai  

* doc/invoke.texi: Add documentation for new option.
* tree-dump.c (dump_begin): Handle stderr appropriately.
(dump_end): Likewise.
(dump_switch_p_1): Likewise.
* tree-pass.h (enum tree_dump_index): Add new constant.
* testsuite/g++.dg/other/dump-stderr-1.C: New test.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 187265)
+++ doc/invoke.texi (working copy)
@@ -5322,20 +5322,23 @@ Here are some examples showing uses of these optio
 
 @item -d@var{letters}
 @itemx -fdump-rtl-@var{pass}
+@itemx -fdump-rtl-@var{pass-stderr}
 @opindex d
 Says to make debugging dumps during compilation at times specified by
 @var{letters}.  This is used for debugging the RTL-based passes of the
 compiler.  The file names for most of the dumps are made by appending
 a pass number and a word to the @var{dumpname}, and the files are
-created in the directory of the output file.  Note that the pass
-number is computed statically as passes get registered into the pass
-manager.  Thus the numbering is not related to the dynamic order of
-execution of passes.  In particular, a pass installed by a plugin
-could have a number over 200 even if it executed quite early.
-@var{dumpname} is generated from the name of the output file, if
-explicitly specified and it is not an executable, otherwise it is the
-basename of the source file. These switches may have different effects
-when @option{-E} is used for preprocessing.
+created in the directory of the output file. If the @option{-stderr} is
+appended to the longer form of the dump option then the dump is
+done on stderr instead of named files. Note that the pass number is
+computed statically as passes get registered into the pass manager.
+Thus the numbering is not related to the dynamic order of execution of
+passes.  In particular, a pass installed by a plugin could have a
+number over 200 even if it executed quite early.  @var{dumpname} is
+generated from the name of the output file, if explicitly specified
+and it is not an executable, otherwise it is the basename of the
+source file. These switches may have different effects when
+@option{-E} is used for preprocessing.
 
 Debug dumps can be enabled with a @option{-fdump-rtl} switch or some
 @option{-d} option @var{letters}.  Here are the possible
@@ -5599,6 +5602,10 @@ These dumps are defined but always produce empty f
 @opindex fdump-rtl-all
 Produce all the dumps listed above.
 
+@item -fdump-rtl-all-stderr
+@opindex fdump-rtl-all-stderr
+Produce all the dumps on stderr.
+
 @item -dA
 @opindex dA
 Annotate the assembler output with miscellaneous debugging information.
@@ -5723,11 +5730,13 @@ counters for each function compiled.
 Control the dumping at various stages of processing the intermediate
 language tree to a file.  The file name is generated by appending a
 switch specific suffix to the source file name, and the file is
-created in the same directory as the output file.  If the
-@samp{-@var{options}} form is used, @var{options} is a list of
-@samp{-} separated options which control the details of the dump.  Not
-all options are applicable to all dumps; those that are not
-meaningful are ignored.  The following options are available
+created in the same directory as the output file unless the option
+@option{stderr} is given.  In case of @option{stderr}, the dump output is on
+the stderr. If the @samp{-@var{options}} form is used, @var{options}
+is a list of @samp{-} separated options which control the details or
+location of the dump.  Not all options are applicable to all dumps;
+those that are not meaningful are ignored.  The following options are
+available
 
 @table @samp
 @item address
@@ -5765,9 +5774,42 @@ Enable showing the tree dump for each statement.
 Enable showing the EH region number holding each statement.
 @item scev
 Enable showing scalar evolution analysis details.
+@item slim
+Inhibit dumping of members of a scope or body of a function merely
+because that scope has been reached.  Only dump such items when they
+are directly reachable by some other path.  When dumping pretty-printed
+trees, this option inhibits dumping the bodies of control structures.
+@item stderr
+Instead of dumping into a named file, dump on to stderr. However, note
+that the @option{stderr} is considered weak, so in case of two
+conflicting dump options, the option to dump into file takes
+precedence regardless of the command line ordering. For instance

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-05-08 Thread Sharad Singhai
That is certainly a possibility. The original motivation was to
implement -fopt-info correctly. If there are other use cases, then I
can enhance the patch.

Thanks,
Sharad


On Mon, May 7, 2012 at 3:02 PM, Gabriel Dos Reis
 wrote:
> On Mon, May 7, 2012 at 4:58 PM, Sharad Singhai  wrote:
>> This is the first patch for planned improvements to dump
>> infrastructure.  Please reference the discussion in
>> http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02088.html.
>>
>> The following small patch allows selective tree and rtl dumps on
>> stderr instead of named files.  Later -fopt-info can be implemented in
>> form of -fdump-xxx-stderr.
>
> Instead of -fdump-xxx-stderr, it will be better to have
>
>    -fdump-xxx=yyy
>
> where yyy is a path to a file, with "stderr" and "stdout" having
> special meaning.
>
> -- Gaby


[PATCH] Add option for dumping to stderr (issue6190057)

2012-05-08 Thread Sharad Singhai
In response to comments, I have updated the patch to support dumps in
user provided files via the option -fdump-xxx=. The
filenames stdout/stderr are treated specially, and are considered
standard streams.

Also updated documentation and a testcase. Okay for trunk?

Thanks,
Sharad

2012-05-08   Sharad Singhai  

* doc/invoke.texi: Add documentation for new option.
* tree-dump.c (dump_stream_p): New function.
(dump_files): Update for new field.
(dump_switch_p_1): Handle user provided filenames.
(dump_begin): Likewise.
(get_dump_file_name): Likewise.
(dump_enable_all): Add new parameter USER_FILENAME.
All callers updated.
(dump_end): Remove attribute.
* tree-pass.h (enum tree_dump_index): Add new constant.
(struct dump_file_info): Add new field USER_FILENAME.
* testsuite/g++.dg/other/dump-userfile-1.C: New test.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 187265)
+++ doc/invoke.texi (working copy)
@@ -5322,20 +5322,24 @@ Here are some examples showing uses of these optio
 
 @item -d@var{letters}
 @itemx -fdump-rtl-@var{pass}
+@itemx -fdump-rtl-@var{pass}=@var{filename}
 @opindex d
 Says to make debugging dumps during compilation at times specified by
 @var{letters}.  This is used for debugging the RTL-based passes of the
 compiler.  The file names for most of the dumps are made by appending
 a pass number and a word to the @var{dumpname}, and the files are
-created in the directory of the output file.  Note that the pass
-number is computed statically as passes get registered into the pass
-manager.  Thus the numbering is not related to the dynamic order of
-execution of passes.  In particular, a pass installed by a plugin
-could have a number over 200 even if it executed quite early.
-@var{dumpname} is generated from the name of the output file, if
-explicitly specified and it is not an executable, otherwise it is the
-basename of the source file. These switches may have different effects
-when @option{-E} is used for preprocessing.
+created in the directory of the output file. If the
+@option{=@var{filename}} is appended to the longer form of the dump
+option then the dump is done on that file instead of numbered
+files. The filenames stdout and stderr are treated specially. Note
+that the pass number is computed statically as passes get registered
+into the pass manager.  Thus the numbering is not related to the
+dynamic order of execution of passes.  In particular, a pass installed
+by a plugin could have a number over 200 even if it executed quite
+early.  @var{dumpname} is generated from the name of the output file,
+if explicitly specified and it is not an executable, otherwise it is
+the basename of the source file. These switches may have different
+effects when @option{-E} is used for preprocessing.
 
 Debug dumps can be enabled with a @option{-fdump-rtl} switch or some
 @option{-d} option @var{letters}.  Here are the possible
@@ -5599,6 +5603,10 @@ These dumps are defined but always produce empty f
 @opindex fdump-rtl-all
 Produce all the dumps listed above.
 
+@item -fdump-rtl-all=stderr
+@opindex fdump-rtl-all=stderr
+Produce all RTL dumps on stderr.
+
 @item -dA
 @opindex dA
 Annotate the assembler output with miscellaneous debugging information.
@@ -5719,15 +5727,19 @@ counters for each function compiled.
 
 @item -fdump-tree-@var{switch}
 @itemx -fdump-tree-@var{switch}-@var{options}
+@itemx -fdump-tree-@var{switch}-@var{options}=@var{filename}
 @opindex fdump-tree
 Control the dumping at various stages of processing the intermediate
 language tree to a file.  The file name is generated by appending a
 switch specific suffix to the source file name, and the file is
-created in the same directory as the output file.  If the
-@samp{-@var{options}} form is used, @var{options} is a list of
-@samp{-} separated options which control the details of the dump.  Not
-all options are applicable to all dumps; those that are not
-meaningful are ignored.  The following options are available
+created in the same directory as the output file. In case of
+@option{=@var{filename}}, the dump output is on the given file. Note
+that the filenames stdout and stderr are treated specially and dumps
+are done on standard streams. If the @samp{-@var{options}} form is
+used, @var{options} is a list of @samp{-} separated options which
+control the details or location of the dump.  Not all options are
+applicable to all dumps; those that are not meaningful are ignored.
+The following options are available
 
 @table @samp
 @item address
@@ -5765,9 +5777,56 @@ Enable showing the tree dump for each statement.
 Enable showing the EH region number holding each statement.
 @item scev
 Enable showing scalar evolution analysis details.
+@item slim
+Inhibit dumping of members of a scope or body of a function merely
+because that scope has been

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-05-09 Thread Sharad Singhai
Thanks for your suggestions/comments. I have updated the patch and
documentation. It supports the following usage:

gcc  -fdump-tree-all=tree.dump -fdump-tree-pre=stdout
-fdump-rtl-ira=ira.dump

Here all tree dumps except the PRE are output into tree.dump, PRE dump
goes to stdout and the IRA dump goes to ira.dump.

Thanks,
Sharad

2012-05-09   Sharad Singhai  

* doc/invoke.texi: Add documentation for the new option.
* tree-dump.c (dump_get_standard_stream): New function.
(dump_files): Update for new field.
(dump_switch_p_1): Handle dump filenames.
(dump_begin): Likewise.
(get_dump_file_name): Likewise.
(dump_end): Remove attribute.
(dump_enable_all): Add new parameter FILENAME.
All callers updated.
(enable_rtl_dump_file):
* tree-pass.h (enum tree_dump_index): Add new constant.
(struct dump_file_info): Add new field FILENAME.
* testsuite/g++.dg/other/dump-filename-1.C: New test.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 187265)
+++ doc/invoke.texi (working copy)
@@ -5322,20 +5322,23 @@ Here are some examples showing uses of these optio

 @item -d@var{letters}
 @itemx -fdump-rtl-@var{pass}
+@itemx -fdump-rtl-@var{pass}=@var{filename}
 @opindex d
 Says to make debugging dumps during compilation at times specified by
 @var{letters}.  This is used for debugging the RTL-based passes of the
 compiler.  The file names for most of the dumps are made by appending
 a pass number and a word to the @var{dumpname}, and the files are
-created in the directory of the output file.  Note that the pass
-number is computed statically as passes get registered into the pass
-manager.  Thus the numbering is not related to the dynamic order of
-execution of passes.  In particular, a pass installed by a plugin
-could have a number over 200 even if it executed quite early.
-@var{dumpname} is generated from the name of the output file, if
-explicitly specified and it is not an executable, otherwise it is the
-basename of the source file. These switches may have different effects
-when @option{-E} is used for preprocessing.
+created in the directory of the output file. If the
+@option{=@var{filename}} is appended to the longer form of the dump
+option then the dump is done on that file instead of numbered
+files. Note that the pass number is computed statically as passes get
+registered into the pass manager.  Thus the numbering is not related
+to the dynamic order of execution of passes.  In particular, a pass
+installed by a plugin could have a number over 200 even if it executed
+quite early.  @var{dumpname} is generated from the name of the output
+file, if explicitly specified and it is not an executable, otherwise
+it is the basename of the source file. These switches may have
+different effects when @option{-E} is used for preprocessing.

 Debug dumps can be enabled with a @option{-fdump-rtl} switch or some
 @option{-d} option @var{letters}.  Here are the possible
@@ -5719,15 +5722,18 @@ counters for each function compiled.

 @item -fdump-tree-@var{switch}
 @itemx -fdump-tree-@var{switch}-@var{options}
+@itemx -fdump-tree-@var{switch}-@var{options}=@var{filename}
 @opindex fdump-tree
 Control the dumping at various stages of processing the intermediate
 language tree to a file.  The file name is generated by appending a
 switch specific suffix to the source file name, and the file is
-created in the same directory as the output file.  If the
-@samp{-@var{options}} form is used, @var{options} is a list of
-@samp{-} separated options which control the details of the dump.  Not
-all options are applicable to all dumps; those that are not
-meaningful are ignored.  The following options are available
+created in the same directory as the output file. In case of
+@option{=@var{filename}} option, the dump is output on the given file
+name instead.  If the @samp{-@var{options}} form is used,
+@var{options} is a list of @samp{-} separated options which control
+the details or location of the dump.  Not all options are applicable
+to all dumps; those that are not meaningful are ignored.  The
+following options are available

 @table @samp
 @item address
@@ -5765,9 +5771,46 @@ Enable showing the tree dump for each statement.
 Enable showing the EH region number holding each statement.
 @item scev
 Enable showing scalar evolution analysis details.
+@item slim
+Inhibit dumping of members of a scope or body of a function merely
+because that scope has been reached.  Only dump such items when they
+are directly reachable by some other path.  When dumping pretty-printed
+trees, this option inhibits dumping the bodies of control structures.
+@item =@var{filename}
+Instead of using an auto generated dump file name, use the given file
+name. The file names @file{stdout} and @file{stderr} are treated
+specially and are considered already open standard

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-05-10 Thread Sharad Singhai
Okay, I have restored the original behavior where standard streams
were considered weak. Thus in case of a conflict, the
standard streams have lower precedence. For example,

   gcc -O2 -fdump-tree-pre=stdout -fdump-tree-pre ...

does the PRE dump in auto numbered file since stdout has lower
precedence. Also this works as expected,

   gcc -O2 -fdump-tree-pre=pre.txt -fdump-tree-all=stderr ...

It outputs PRE dump to pre.txt while the remaining tree dumps are
output on to stderr. Does it look okay?

Thanks,
Sharad


2012-05-09   Sharad Singhai  

* doc/invoke.texi: Add documentation for the new option.
* tree-dump.c (dump_get_standard_stream): New function.
(dump_files): Update for new field.
(dump_switch_p_1): Handle dump filenames.
(dump_begin): Likewise.
(get_dump_file_name): Likewise.
(dump_end): Remove attribute.
(dump_enable_all): Add new parameter FILENAME.
All callers updated.
* tree-pass.h (enum tree_dump_index): Add new constant.
(struct dump_file_info): Add new field FILENAME.
* testsuite/g++.dg/other/dump-filename-1.C: New test.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 187265)
+++ doc/invoke.texi (working copy)
@@ -5322,20 +5322,23 @@ Here are some examples showing uses of these optio

 @item -d@var{letters}
 @itemx -fdump-rtl-@var{pass}
+@itemx -fdump-rtl-@var{pass}=@var{filename}
 @opindex d
 Says to make debugging dumps during compilation at times specified by
 @var{letters}.  This is used for debugging the RTL-based passes of the
 compiler.  The file names for most of the dumps are made by appending
 a pass number and a word to the @var{dumpname}, and the files are
-created in the directory of the output file.  Note that the pass
-number is computed statically as passes get registered into the pass
-manager.  Thus the numbering is not related to the dynamic order of
-execution of passes.  In particular, a pass installed by a plugin
-could have a number over 200 even if it executed quite early.
-@var{dumpname} is generated from the name of the output file, if
-explicitly specified and it is not an executable, otherwise it is the
-basename of the source file. These switches may have different effects
-when @option{-E} is used for preprocessing.
+created in the directory of the output file. If the
+@option{=@var{filename}} is appended to the longer form of the dump
+option then the dump is done on that file instead of numbered
+files. Note that the pass number is computed statically as passes get
+registered into the pass manager.  Thus the numbering is not related
+to the dynamic order of execution of passes.  In particular, a pass
+installed by a plugin could have a number over 200 even if it executed
+quite early.  @var{dumpname} is generated from the name of the output
+file, if explicitly specified and it is not an executable, otherwise
+it is the basename of the source file. These switches may have
+different effects when @option{-E} is used for preprocessing.

 Debug dumps can be enabled with a @option{-fdump-rtl} switch or some
 @option{-d} option @var{letters}.  Here are the possible
@@ -5719,15 +5722,18 @@ counters for each function compiled.

 @item -fdump-tree-@var{switch}
 @itemx -fdump-tree-@var{switch}-@var{options}
+@itemx -fdump-tree-@var{switch}-@var{options}=@var{filename}
 @opindex fdump-tree
 Control the dumping at various stages of processing the intermediate
 language tree to a file.  The file name is generated by appending a
 switch specific suffix to the source file name, and the file is
-created in the same directory as the output file.  If the
-@samp{-@var{options}} form is used, @var{options} is a list of
-@samp{-} separated options which control the details of the dump.  Not
-all options are applicable to all dumps; those that are not
-meaningful are ignored.  The following options are available
+created in the same directory as the output file. In case of
+@option{=@var{filename}} option, the dump is output on the given file
+name instead.  If the @samp{-@var{options}} form is used,
+@var{options} is a list of @samp{-} separated options which control
+the details or location of the dump.  Not all options are applicable
+to all dumps; those that are not meaningful are ignored.  The
+following options are available

 @table @samp
 @item address
@@ -5765,9 +5771,49 @@ Enable showing the tree dump for each statement.
 Enable showing the EH region number holding each statement.
 @item scev
 Enable showing scalar evolution analysis details.
+@item slim
+Inhibit dumping of members of a scope or body of a function merely
+because that scope has been reached.  Only dump such items when they
+are directly reachable by some other path.  When dumping pretty-printed
+trees, this option inhibits dumping the bodies of control structures.
+@item =@var{filename}
+Instead of

Re: [PATCH] Add option for dumping to stderr (issue6190057)

2012-06-12 Thread Sharad Singhai
Ping.

Thanks,
Sharad

On Thu, Jun 7, 2012 at 10:16 PM, Sharad Singhai  wrote:
> Okay, I have updated the attached patch so that the output from
> -ftree-vectorizer-verbose is considered diagnostic information and is
> always
> sent to stderr. Other functionality remains unchanged. Here is some
> more context about this patch.
>
> This patch improves the dump infrastructure and public interfaces so
> that the existing private pass-specific dump stream is separated from
> the diagnostic dump stream (typically stderr).  The optimization
> passes can output information on the two streams independently.
>
> The newly defined interfaces are:
>
> Individual passes do not need to access the dump file directly. Thus Instead
> of doing
>
>   if (dump_file && (flags & dump_flags))
>      fprintf (dump_file, ...);
>
> they can do
>
>     dump_printf (flags, ...);
>
> If the current pass has FLAGS enabled then the information gets
> printed into the dump file otherwise not.
>
> Similar to the dump_printf (), another function is defined, called
>
>        diag_printf (dump_flags, ...)
>
> This prints information only onto the diagnostic stream, typically
> standard error. It is useful for separating pass-specific dump
> information from
> the diagnostic information.
>
> Currently, as a proof of concept, I have converted vectorizer passes
> to use the new dump format. For this, I have considered
> information printed in vect_dump file as diagnostic. Thus 'fprintf'
> calls are changed to 'diag_printf'. Some other information printed to
> dump_file is sent to the regular dump file via 'dump_printf ()'. It
> helps to separate the two streams because they might serve different
> purposes and might have different formatting requirements.
>
> For example, using the trunk compiler, the following invocation
>
> g++ -S v.cc -ftree-vectorize -fdump-tree-vect -ftree-vectorizer-verbose=2
>
> prints tree vectorizer dump into a file named 'v.cc.113t.vect'.
> However, the verbose diagnostic output is silently
> ignored. This is not desirable as the two types of dump should not interfere.
>
> After this patch, the vectorizer dump is available in 'v.cc.113t.vect'
> as before, but the verbose vectorizer diagnostic is additionally
> printed on stderr. Thus both types of dump information are output.
>
> An additional feature of this patch is that individual passes can
> print dump information into command-line named files instead of auto
> numbered filename. For example,
>
> g++ -S -O2 v.cc -ftree-vectorize -fdump-tree-vect=foo.vect
>     -ftree-vectorizer-verbose=2
>     -fdump-tree-pre=foo.pre
>
> This prints the tree vectorizer dump into 'foo.vect', PRE dump into
> 'foo.pre', and the vectorizer verbose diagnostic dump onto stderr.
>
> Please take another look.
>
> Thanks,
> Sharad
>
>
> On Thu, Jun 7, 2012 at 12:19 AM, Xinliang David Li  wrote:
>> On Wed, Jun 6, 2012 at 10:58 PM, Sharad Singhai  wrote:
>>> Sorry about the delay. I have finally incorporated all the suggestions
>>> and reorganized the dump infrastructure a bit. The attached patch
>>> updates vectorizer passes so that instead of accessing global
>>> dump_file directly, these passes call dump_printf (FLAG, format, ...).
>>> The dump_printf can choose between two streams, one regular pass dump
>>> file, and another optional command line provided file. Currently, it
>>> doesn't discriminate and all the dump information goes to both the
>>> streams.
>>>
>>> Thus, for example,
>>>
>>> g++ -O2 v.cc -ftree-vectorize -fdump-tree-vect=foo.v 
>>> -ftree-vectorizer-verbose=3
>>
>> But the default form of dump option (-fdump-tree-vect) no longer
>> interferes with -ftree-vectorize-verbose=xxx output right? (this is
>> one of the main requirements). One dumped to the primary stream (named
>> dump file) and the other to the stderr -- the default diagnostic (alt)
>> stream.
>>
>> David
>>
>>>
>>> will output the verbose vectorizer information in both *.vect file and
>>> foo.v file. However, as I have converted only vectorizer passes so
>>> far, there is additional information in *.vect file which is not
>>> present in foo.v file. Once other passes are converted to use this
>>> scheme, then these two dump files should have identical output.
>>>
>>> Also note that in this patch -fdump-xxx=yyy format does not override
>>> any auto named dump files as in my earlier patches. Instead the dump
>>> information is output to both pl

Disable early inlining while compiling for coverage (issue5173042)

2011-09-30 Thread Sharad Singhai
This patch disables early inlining when --coverage option is
specified. This improves coverage data in presence of other
optimizations, specially with -O2 where early inlining changes the
control flow graph sufficiently enough to generate seemingly very odd
source coverage.

Bootstrapped okay and regression tests passed.

Okay for google/gcc-4_6?

2011-09-30   Sharad Singhai  

* gcc.c (cc1_options): Added -fno-early-inlining for coverage.

Index: gcc.c
===
--- gcc.c   (revision 179402)
+++ gcc.c   (working copy)
@@ -776,7 +776,7 @@
  %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\
  %{fsyntax-only:-o %j} %{-param*}\
  %{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\
- %{coverage:-fprofile-arcs -ftest-coverage}";
+ %{coverage:-fprofile-arcs -ftest-coverage -fno-early-inlining}";
 
 /* If an assembler wrapper is used to invoke post-assembly tools
like MAO, --save-temps need to be passed to save the output of

--
This patch is available for review at http://codereview.appspot.com/5173042


Re: [PATCH] Add an intermediate coverage format for gcov

2011-10-05 Thread Sharad Singhai
Sorry about the badly formatted mail. Here is another version with a
different mailer. -Sharad


This patch was earlier submitted to google/main, but I propose it
for the trunk as well.

This patch adds an intermediate coverage format (enabled via 'gcov
-i'). This is a compact format as it does not require source files.

The new option ('gcov -i') outputs .gcov files in an intermediate text
format that can be postprocessed by lcov or other applications. It
will output a single *.gcov file per *.gcda file. No source code is
required.

The format of the intermediate .gcov file is plain text with one entry
per line

SF:source_file_name
FN:line_number,function_name
FNDA:execution_count,function_name
BA:line_num,branch_coverage_type
DA:line number,execution_count

Where the branch_coverage_type is
   0 (Branch not executed)
   1 (Branch executed, but not taken)
   2 (Branch executed and taken)

There can be multiple SF entries in an intermediate gcov file. All
entries following SF pertain to that source file until the next SF
entry.

A concrete example looks like this:

  SF:array.c
  FN:4,sum
  FNDA:0,sum
  FN:13,main
  FNDA:1,main
  DA:4,2
  BA:8,2
  DA:7,1

I have bootstrapped and tested this patch on x86_64-linux. No new test
failures were observed. 

Okay for trunk?

Thanks,
Sharad


2011-10-04   Sharad Singhai  

* doc/gcov.texi: Document gcov intermediate format.
* gcov.c (print_usage): Handle new option.
(process_args): Handle new option.
(get_gcov_file_intermediate_name): New function.
(output_intermediate_file): New function.
(generate_results): Handle new option.
* testsuite/lib/gcov.exp: Handle intermediate format.
* testsuite/g++.dg/gcov/gcov-8.C: New testcase.

Index: doc/gcov.texi
===
--- doc/gcov.texi   (revision 179475)
+++ doc/gcov.texi   (working copy)
@@ -130,6 +130,7 @@ gcov [@option{-v}|@option{--version}] [@
  [@option{-f}|@option{--function-summaries}]
  [@option{-o}|@option{--object-directory} @var{directory|file}] 
@var{sourcefiles}
  [@option{-u}|@option{--unconditional-branches}]
+ [@option{-i}|@option{--intermediate-format}]
  [@option{-d}|@option{--display-progress}]
 @c man end
 @c man begin SEEALSO
@@ -216,6 +217,32 @@ Unconditional branches are normally not 
 @itemx --display-progress
 Display the progress on the standard output.
 
+@item -i
+@itemx --intermediate-format
+Output gcov file in an intermediate text format that can be used by
+@command{lcov} or other applications. It will output a single *.gcov file per
+*.gcda file. No source code is required.
+
+The format of the intermediate @file{.gcov} file is plain text with
+one entry per line
+
+@smallexample
+SF:@var{source_file_name}
+FN:@var{line_number},@var{function_name}
+FNDA:@var{execution_count},@var{function_name}
+BA:@var{line_num},@var{branch_coverage_type}
+DA:@var{line number},@var{execution_count}
+
+Where the @var{branch_coverage_type} is
+   0 (Branch not executed)
+   1 (Branch executed, but not taken)
+   2 (Branch executed and taken)
+
+There can be multiple SF entries in an intermediate gcov file. All
+entries following SF pertain to that source file until the next SF
+entry.
+@end smallexample
+
 @end table
 
 @command{gcov} should be run with the current directory the same as that
Index: gcov.c
===
--- gcov.c  (revision 179475)
+++ gcov.c  (working copy)
@@ -39,6 +39,7 @@ along with Gcov; see the file COPYING3. 
 #include "intl.h"
 #include "diagnostic.h"
 #include "version.h"
+#include "demangle.h"
 
 #include 
 
@@ -311,6 +312,9 @@ static int flag_gcov_file = 1;
 
 static int flag_display_progress = 0;
 
+/* Output *.gcov file in intermediate format used by 'lcov'.  */
+static int flag_intermediate_format = 0;
+
 /* For included files, make the gcov output file name include the name
of the input source file.  For example, if x.h is included in a.c,
then the output file name is a.c##x.h.gcov instead of x.h.gcov.  */
@@ -436,6 +440,11 @@ print_usage (int error_p)
   fnotice (file, "  -o, --object-directory DIR|FILE Search for object files in 
DIR or called FILE\n");
   fnotice (file, "  -p, --preserve-pathsPreserve all pathname 
components\n");
   fnotice (file, "  -u, --unconditional-branchesShow unconditional branch 
counts too\n");
+  fnotice (file, "  -i, --intermediate-format   Output .gcov file in an 
intermediate text\n\
+format that can be used by 'lcov' or 
other\n\
+applications.  It will output a single\n\
+.gcov file per .gcda file.  No source 
file\n\
+   

[google] Improve locus information during if-conversion (issue4526101)

2011-06-01 Thread Sharad Singhai
This patch improves locus information during the if-conversion. Okay for 
google/main?

Thanks,
Sharad

2011-06-01  Sharad Singhai  

Google Ref 39994
* ifcvt.c (noce_try_cmove_arith): Use the locus information
from the if-statment rather than the then path.

Index: ifcvt.c
===
--- ifcvt.c (revision 174547)
+++ ifcvt.c (working copy)
@@ -1670,7 +1670,7 @@
   if (!tmp)
 return FALSE;
 
-  emit_insn_before_setloc (tmp, if_info->jump, INSN_LOCATOR (if_info->insn_a));
+  emit_insn_before_setloc (tmp, if_info->jump, INSN_LOCATOR (if_info->jump));
   return TRUE;
 
  end_seq_and_fail:

--
This patch is available for review at http://codereview.appspot.com/4526101


[google] Add intermediate text format for gcov (issue4595053)

2011-06-13 Thread Sharad Singhai
This patch adds an intermediate gcov text format which does not require
source code. This format can be used by lcov or other tools.

I have bootstrapped it on x86 and all tests pass. Okay for main?

Thanks,
Sharad

2011-06-13   Sharad Singhai  

Google Ref 3

* doc/gcov.texi: Document gcov intermediate format.
* gcov.c (get_gcov_file_intermediate_name): New function.
(output_intermediate_file): New function.
* testsuite/g++.dg/gcov/gcov-7.C: New test.


Index: doc/gcov.texi
===
--- doc/gcov.texi   (revision 174926)
+++ doc/gcov.texi   (working copy)
@@ -130,6 +130,7 @@
  [@option{-f}|@option{--function-summaries}]
  [@option{-o}|@option{--object-directory} @var{directory|file}] 
@var{sourcefiles}
  [@option{-u}|@option{--unconditional-branches}]
+ [@option{-i}|@option{--intermediate-format}]
  [@option{-d}|@option{--display-progress}]
 @c man end
 @c man begin SEEALSO
@@ -216,6 +217,12 @@
 @itemx --display-progress
 Display the progress on the standard output.
 
+@item -i
+@itemx --intermediate-format
+Output gcov file in an intermediate text format that can be used by
+@command{lcov} or other applications. It will output a single *.gcov file per
+*gcda file. No source code is required.
+
 @end table
 
 @command{gcov} should be run with the current directory the same as that
Index: gcov.c
===
--- gcov.c  (revision 174926)
+++ gcov.c  (working copy)
@@ -38,6 +38,7 @@
 #include "tm.h"
 #include "intl.h"
 #include "version.h"
+#include "demangle.h"
 
 #include 
 
@@ -310,6 +311,9 @@
 
 static int flag_display_progress = 0;
 
+/* Output *.gcov file in intermediate format used by 'lcov'.  */
+static int flag_intermediate_format = 0;
+
 /* For included files, make the gcov output file name include the name
of the input source file.  For example, if x.h is included in a.c,
then the output file name is a.c##x.h.gcov instead of x.h.gcov.  */
@@ -436,6 +440,11 @@
   fnotice (file, "  -o, --object-directory DIR|FILE Search for object files in 
DIR or called FILE\n");
   fnotice (file, "  -p, --preserve-pathsPreserve all pathname 
components\n");
   fnotice (file, "  -u, --unconditional-branchesShow unconditional branch 
counts too\n");
+  fnotice (file, "  -i, --intermediate-format   Output .gcov file in an 
intermediate text\n\
+format that can be used by 'lcov' or 
other\n\
+applications.  It will output a single\n\
+.gcov file per .gcda file.  No source 
file\n\
+is required.\n");
   fnotice (file, "  -d, --display-progress  Display progress 
information\n");
   fnotice (file, "\nFor bug reporting instructions, please see:\n%s.\n",
   bug_report_url);
@@ -472,6 +481,7 @@
   { "object-file",  required_argument, NULL, 'o' },
   { "unconditional-branches", no_argument, NULL, 'u' },
   { "display-progress", no_argument,   NULL, 'd' },
+  { "intermediate-format",  no_argument,   NULL, 'i' },
   { 0, 0, 0, 0 }
 };
 
@@ -482,7 +492,8 @@
 {
   int opt;
 
-  while ((opt = getopt_long (argc, argv, "abcdfhlno:puv", options, NULL)) != 
-1)
+  while ((opt = getopt_long (argc, argv, "abcdfhilno:puv", options, NULL)) !=
+ -1)
 {
   switch (opt)
{
@@ -516,6 +527,10 @@
case 'u':
  flag_unconditional = 1;
  break;
+   case 'i':
+  flag_intermediate_format = 1;
+  flag_gcov_file = 1;
+  break;
 case 'd':
   flag_display_progress = 1;
   break;
@@ -531,6 +546,109 @@
   return optind;
 }
 
+/* Get the name of the gcov file.  The return value must be free'd.
+
+   It appends the '.gcov' extension to the *basename* of the file.
+   The resulting file name will be in PWD.
+
+   e.g.,
+   input: foo.da,   output: foo.da.gcov
+   input: a/b/foo.cc,   output: foo.cc.gcov  */
+
+static char *
+get_gcov_file_intermediate_name (const char *file_name)
+{
+  const char *gcov = ".gcov";
+  char *result;
+  const char *cptr;
+
+  /* Find the 'basename'.  */
+  cptr = lbasename (file_name);
+
+  result = XNEWVEC(char, strlen (cptr) + strlen (gcov) + 1);
+  sprintf (result, "%s%s", cptr, gcov);
+
+  return result;
+}
+
+/* Output the result in intermediate format used by 'lcov'.
+
+This format contains a single file named 'foo.cc.gcov', with no source
+code included.
+
+SF:/home/.../foo.h
+DA:10,1
+DA:30,0
+DA:35,1
+SF:/h

[google] Use different peeling parameters with available profile (issue4438079)

2011-04-27 Thread Sharad Singhai
This patch adds new parameters to control peeling when profile
feedback information is available.

For google/main.

Tested:
  bootstrapped on x86_64.

2011-04-27  Sharad Singhai  

* gcc/params.def: Add new parameters to control peeling.
* gcc/tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use
different peeling parameters when profile feedback is available.
* gcc/loop-unroll.c (decide_peel_once_rolling): Ditto.
(decide_peel_completely): Ditto.
* gcc/doc/invoke.texi: Document new peeling parameters.
* gcc/testsuite/gcc.dg/vect/O3-vect-pr34223.c: Add new peeling
parameters.
* gcc/testsuite/gcc.dg/vect/vect.exp: Allow reading flags in individual
tests.


Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 172904)
+++ gcc/doc/invoke.texi (working copy)
@@ -8523,11 +8523,28 @@
 The maximum number of peelings of a single loop.
 
 @item max-completely-peeled-insns
+@item max-completely-peeled-insns-feedback
 The maximum number of insns of a completely peeled loop.
 
+The @option{max-completely-peeled-insns-feedback} is used only when profile
+feedback is available and the loop is hot. Because of the real profiles, this
+value may set to be larger for hot loops.
+
+@item max-once-peeled-insns
+@item max-once-peeled-insns-feedback
+The maximum number of insns of a peeled loop that rolls only once.
+The @option{max-once-peeled-insns-feedback}  is used only when profile feedback
+is available and the loop is hot. Because of the real profiles, this value
+may set to be larger for hot loops.
+
 @item max-completely-peel-times
+@item max-completely-peel-times-feedback
 The maximum number of iterations of a loop to be suitable for complete peeling.
 
+The @option{max-completely-peel-times-feedback} is used only when profile 
feedback
+is available and the loop is hot. Because of the real profiles, this value may
+set to be larger for hot loops.
+
 @item max-completely-peel-loop-nest-depth
 The maximum depth of a loop nest suitable for complete peeling.
 
Index: gcc/testsuite/gcc.dg/vect/O3-vect-pr34223.c
===
--- gcc/testsuite/gcc.dg/vect/O3-vect-pr34223.c (revision 172904)
+++ gcc/testsuite/gcc.dg/vect/O3-vect-pr34223.c (working copy)
@@ -1,4 +1,5 @@
 /* { dg-require-effective-target vect_int } */
+/* { dg-options "[vect_cflags] --param max-completely-peel-times=16" } */
 
 #include "tree-vect.h"
 
Index: gcc/testsuite/gcc.dg/vect/vect.exp
===
--- gcc/testsuite/gcc.dg/vect/vect.exp  (revision 172904)
+++ gcc/testsuite/gcc.dg/vect/vect.exp  (working copy)
@@ -24,6 +24,12 @@
 global DEFAULT_VECTCFLAGS
 set DEFAULT_VECTCFLAGS ""
 
+# So that we can read flags in individual tests.
+proc vect_cflags { } {
+  global DEFAULT_VECTCFLAGS
+  return $DEFAULT_VECTCFLAGS
+}
+
 # If the target system supports vector instructions, the default action
 # for a test is 'run', otherwise it's 'compile'.  Save current default.
 # Executing vector instructions on a system without hardware vector support
Index: gcc/tree-ssa-loop-ivcanon.c
===
--- gcc/tree-ssa-loop-ivcanon.c (revision 172904)
+++ gcc/tree-ssa-loop-ivcanon.c (working copy)
@@ -326,6 +326,7 @@
enum unroll_level ul)
 {
   unsigned HOST_WIDE_INT n_unroll, ninsns, max_unroll, unr_insns;
+  unsigned HOST_WIDE_INT max_peeled_insns;
   gimple cond;
   struct loop_size size;
 
@@ -336,9 +337,21 @@
 return false;
   n_unroll = tree_low_cst (niter, 1);
 
-  max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES);
-  if (n_unroll > max_unroll)
+  if (profile_info && flag_branch_probabilities &&
+  optimize_loop_for_speed_p (loop))
+max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES_FEEDBACK);
+  else
+max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES);
+
+  if (n_unroll > max_unroll) {
+if (dump_file && (dump_flags & TDF_DETAILS))
+   {
+ fprintf (dump_file, "  Not unrolling loop %d limited by max unroll"
+   " (%d > %d)\n",
+   loop->num, (int) n_unroll, (int) max_unroll);
+}
 return false;
+  }
 
   if (n_unroll)
 {
@@ -356,14 +369,20 @@
   (int) unr_insns);
}
 
-  if (unr_insns > ninsns
- && (unr_insns
- > (unsigned) PARAM_VALUE (PARAM_MAX_COMPLETELY_PEELED_INSNS)))
+  if (profile_info && flag_branch_probabilities &&
+  optimize_loop_for_speed_p (loop))
+max_peeled_insns =
+  PARAM_VALUE (PARAM_MAX_COMPLETELY_PEELED_INSNS_FEEDBACK);
+  else
+max_peeled_insns = PARAM_VAL

Here is an updated patch. (issue4438079)

2011-04-27 Thread Sharad Singhai
Hi Diego,

Thanks for the quick feedback. Here is a an updated version of the patch.

2011-04-27  Sharad Singhai  

ChangeLog.google-main
* params.def: Add new parameters to control peeling.
* tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use
different peeling parameters when profile feedback is available.
* loop-unroll.c (decide_peel_once_rolling): Ditto.
(decide_peel_completely): Ditto.
* doc/invoke.texi: Document new peeling parameters.

testsuite/ChangeLog.google-main
* gcc.dg/vect/O3-vect-pr34223.c: Add new peeling
parameters.
* gcc.dg/vect/vect.exp: Allow reading flags in individual
tests.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 172904)
+++ doc/invoke.texi (working copy)
@@ -8523,11 +8523,28 @@
 The maximum number of peelings of a single loop.
 
 @item max-completely-peeled-insns
+@item max-completely-peeled-insns-feedback
 The maximum number of insns of a completely peeled loop.
 
+The @option{max-completely-peeled-insns-feedback} is used only when profile
+feedback is available and the loop is hot. Because of the real profiles, this
+value may set to be larger for hot loops.
+
+@item max-once-peeled-insns
+@item max-once-peeled-insns-feedback
+The maximum number of insns of a peeled loop that rolls only once.
+The @option{max-once-peeled-insns-feedback}  is used only when profile feedback
+is available and the loop is hot. Because of the real profiles, this value
+may set to be larger for hot loops.
+
 @item max-completely-peel-times
+@item max-completely-peel-times-feedback
 The maximum number of iterations of a loop to be suitable for complete peeling.
 
+The @option{max-completely-peel-times-feedback} is used only when profile 
feedback
+is available and the loop is hot. Because of the real profiles, this value may
+set to be larger for hot loops.
+
 @item max-completely-peel-loop-nest-depth
 The maximum depth of a loop nest suitable for complete peeling.
 
Index: testsuite/gcc.dg/vect/O3-vect-pr34223.c
===
--- testsuite/gcc.dg/vect/O3-vect-pr34223.c (revision 172904)
+++ testsuite/gcc.dg/vect/O3-vect-pr34223.c (working copy)
@@ -1,4 +1,5 @@
 /* { dg-require-effective-target vect_int } */
+/* { dg-options "[vect_cflags] --param max-completely-peel-times=16" } */
 
 #include "tree-vect.h"
 
Index: testsuite/gcc.dg/vect/vect.exp
===
--- testsuite/gcc.dg/vect/vect.exp  (revision 172904)
+++ testsuite/gcc.dg/vect/vect.exp  (working copy)
@@ -24,6 +24,12 @@
 global DEFAULT_VECTCFLAGS
 set DEFAULT_VECTCFLAGS ""
 
+# So that we can read flags in individual tests.
+proc vect_cflags { } {
+  global DEFAULT_VECTCFLAGS
+  return $DEFAULT_VECTCFLAGS
+}
+
 # If the target system supports vector instructions, the default action
 # for a test is 'run', otherwise it's 'compile'.  Save current default.
 # Executing vector instructions on a system without hardware vector support
Index: tree-ssa-loop-ivcanon.c
===
--- tree-ssa-loop-ivcanon.c (revision 172904)
+++ tree-ssa-loop-ivcanon.c (working copy)
@@ -326,6 +326,7 @@
enum unroll_level ul)
 {
   unsigned HOST_WIDE_INT n_unroll, ninsns, max_unroll, unr_insns;
+  unsigned HOST_WIDE_INT max_peeled_insns;
   gimple cond;
   struct loop_size size;
 
@@ -336,9 +337,23 @@
 return false;
   n_unroll = tree_low_cst (niter, 1);
 
-  max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES);
+  if (profile_info
+  && flag_branch_probabilities
+  && optimize_loop_for_speed_p (loop))
+max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES_FEEDBACK);
+  else
+max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES);
+
   if (n_unroll > max_unroll)
+{
+  if (dump_file && (dump_flags & TDF_DETAILS))
+   {
+ fprintf (dump_file, "  Not unrolling loop %d limited by max unroll"
+   " (%d > %d)\n",
+   loop->num, (int) n_unroll, (int) max_unroll);
+}
 return false;
+  }
 
   if (n_unroll)
 {
@@ -356,14 +371,21 @@
   (int) unr_insns);
}
 
-  if (unr_insns > ninsns
- && (unr_insns
- > (unsigned) PARAM_VALUE (PARAM_MAX_COMPLETELY_PEELED_INSNS)))
+  if (profile_info
+  && flag_branch_probabilities
+  && optimize_loop_for_speed_p (loop))
+max_peeled_insns =
+  PARAM_VALUE (PARAM_MAX_COMPLETELY_PEELED_INSNS_FEEDBACK);
+  else
+max_peeled_insns = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEELED_INSNS);
+
+  if (unr_ins

Disable tracer by default for profile use (issue4428074)

2011-04-28 Thread Sharad Singhai
This patch disables -ftracer for profile use. Okay for google/main?

Thanks,
Sharad

2011-04-28  Sharad Singhai  

Google Ref 40087
* opts.c (common_handle_option): Disable -ftracer for profile use.
* doc/invoke.texi: Update doc that -ftracer is no longer enabled for 
FDO.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 173048)
+++ doc/invoke.texi (working copy)
@@ -7930,7 +7930,7 @@
 generally profitable only with profile feedback available.
 
 The following options are enabled: @code{-fbranch-probabilities}, @code{-fvpt},
-@code{-funroll-loops}, @code{-fpeel-loops}, @code{-ftracer}
+@code{-funroll-loops}, @code{-fpeel-loops}.
 
 By default, GCC emits an error message if the feedback profiles do not
 match the source code.  This error can be turned into a warning by using
Index: opts.c
===
--- opts.c  (revision 173048)
+++ opts.c  (working copy)
@@ -1531,8 +1531,6 @@
opts->x_flag_unroll_loops = value;
   if (!opts_set->x_flag_peel_loops)
opts->x_flag_peel_loops = value;
-  if (!opts_set->x_flag_tracer)
-   opts->x_flag_tracer = value;
   if (!opts_set->x_flag_value_profile_transformations)
opts->x_flag_value_profile_transformations = value;
   if (!opts_set->x_flag_inline_functions)

--
This patch is available for review at http://codereview.appspot.com/4428074


[google] Updated patch (issue4438079)

2011-04-28 Thread Sharad Singhai
I have fixed documentation so that new params are documented
separately along with default values. Also used profile_status
variable.

I experimented with scaling defaults but that was not always possible
as some parameters need to be more indpependent.

Thanks,
Sharad

Google Ref 40474

2011-04-28  Sharad Singhai  

gcc/ChangeLog.google-main
* params.def: Add new parameters to control peeling.
* tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use
different peeling parameters when profile feedback is available.
* loop-unroll.c (decide_peel_once_rolling): Ditto.
(decide_peel_completely): Ditto.
* doc/invoke.texi: Document new peeling parameters.

testsuite/ChangeLog.google-main
* gcc.dg/vect/O3-vect-pr34223.c: Add extra peeling
parameters.
* gcc.dg/vect/vect.exp: Allow reading flags in individual
tests.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 173140)
+++ doc/invoke.texi (working copy)
@@ -8634,9 +8634,29 @@
 @item max-completely-peeled-insns
 The maximum number of insns of a completely peeled loop.
 
+@item max-completely-peeled-insns-feedback
+The maximum number of insns of a completely peeled loop when profile
+feedback is available and the loop is hot. Because of the real
+profiles, this value may set to be larger for hot loops. Its default
+value is 600.
+
+@item max-once-peeled-insns
+The maximum number of insns of a peeled loop that rolls only once.
+
+@item max-once-peeled-insns-feedback
+The maximum number of insns of a peeled loop when profile feedback is
+available and the loop is hot. Because of the real profiles, this
+value may set to be larger for hot loops. The default value is 600.
+
 @item max-completely-peel-times
 The maximum number of iterations of a loop to be suitable for complete peeling.
 
+@item max-completely-peel-times-feedback
+The maximum number of iterations of a loop to be suitable for complete
+peeling when profile feedback is available and the loop is
+hot. Because of the real profiles, this value may set to be larger for
+hot loops. Its default value is 16.
+
 @item max-completely-peel-loop-nest-depth
 The maximum depth of a loop nest suitable for complete peeling.
 
Index: testsuite/gcc.dg/vect/O3-vect-pr34223.c
===
--- testsuite/gcc.dg/vect/O3-vect-pr34223.c (revision 173140)
+++ testsuite/gcc.dg/vect/O3-vect-pr34223.c (working copy)
@@ -1,4 +1,5 @@
 /* { dg-require-effective-target vect_int } */
+/* { dg-options "[vect_cflags] --param max-completely-peel-times=16" } */
 
 #include "tree-vect.h"
 
Index: testsuite/gcc.dg/vect/vect.exp
===
--- testsuite/gcc.dg/vect/vect.exp  (revision 173140)
+++ testsuite/gcc.dg/vect/vect.exp  (working copy)
@@ -24,6 +24,12 @@
 global DEFAULT_VECTCFLAGS
 set DEFAULT_VECTCFLAGS ""
 
+# So that we can read flags in individual tests.
+proc vect_cflags { } {
+  global DEFAULT_VECTCFLAGS
+  return $DEFAULT_VECTCFLAGS
+}
+
 # If the target system supports vector instructions, the default action
 # for a test is 'run', otherwise it's 'compile'.  Save current default.
 # Executing vector instructions on a system without hardware vector support
Index: tree-ssa-loop-ivcanon.c
===
--- tree-ssa-loop-ivcanon.c (revision 173140)
+++ tree-ssa-loop-ivcanon.c (working copy)
@@ -326,6 +326,7 @@
enum unroll_level ul)
 {
   unsigned HOST_WIDE_INT n_unroll, ninsns, max_unroll, unr_insns;
+  unsigned HOST_WIDE_INT max_peeled_insns;
   gimple cond;
   struct loop_size size;
 
@@ -336,9 +337,22 @@
 return false;
   n_unroll = tree_low_cst (niter, 1);
 
-  max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES);
+  if (profile_status == PROFILE_READ
+  && optimize_loop_for_speed_p (loop))
+max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES_FEEDBACK);
+  else
+max_unroll = PARAM_VALUE (PARAM_MAX_COMPLETELY_PEEL_TIMES);
+
   if (n_unroll > max_unroll)
+{
+  if (dump_file && (dump_flags & TDF_DETAILS))
+   {
+ fprintf (dump_file, "  Not unrolling loop %d limited by max unroll"
+   " (%d > %d)\n",
+   loop->num, (int) n_unroll, (int) max_unroll);
+}
 return false;
+  }
 
   if (n_unroll)
 {
@@ -356,14 +370,20 @@
   (int) unr_insns);
}
 
-  if (unr_insns > ninsns
- && (unr_insns
- > (unsigned) PARAM_VALUE (PARAM_MAX_COMPLETELY_PEELED_INSNS)))
+  if (profile_status == PROFILE_READ
+  && optimize_loop_for_speed_p (loop))
+max_peeled_in