| Message ID | CAAe5K+UGj-uK=csrHSdd-pa-_Au8Kh0F9-enOuXrLqoNzG_u9Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 16, 2013 at 8:36 PM, Teresa Johnson <tejohnson@google.com> wrote: > Yep, looked too quickly every time and thought the newline after "be > zero" was applying. Here is the patch with the fix. Ok for trunk > pending regression testing? Ok. Thanks, Richard. > 2013-09-16 Teresa Johnson <tejohnson@google.com> > > * coverage.c (get_coverage_counts): Add missing newline. > > Index: coverage.c > =================================================================== > --- coverage.c (revision 202628) > +++ coverage.c (working copy) > @@ -347,7 +347,7 @@ get_coverage_counts (unsigned counter, unsigned ex > if (!warned++ && dump_enabled_p ()) > dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location, > (flag_guess_branch_prob > - ? "file %s not found, execution counts estimated" > + ? "file %s not found, execution counts estimated\n" > : "file %s not found, execution counts assumed to " > "be zero\n"), > da_file_name); > > Thanks, > Teresa > > On Mon, Sep 16, 2013 at 11:20 AM, Xinliang David Li <davidxl@google.com> wrote: >> Looks like there is one missing spot: >> >> @@ -349,7 +349,7 @@ get_coverage_counts (unsigned counter, u >> (flag_guess_branch_prob >> ? "file %s not found, execution counts >> estimated" <---- >> : "file %s not found, execution counts assumed to " >> - "be zero"), >> + "be zero\n"), >> da_file_name); >> return NULL; >> >> >> I found this when testing interaction of -fprofile-use and >> -fno-tree-vectorize without a profile. >> >> thanks, >> >> David >> >> >> On Mon, Sep 16, 2013 at 11:06 AM, Teresa Johnson <tejohnson@google.com> wrote: >>> On Mon, Sep 16, 2013 at 10:57 AM, Xinliang David Li <davidxl@google.com> wrote: >>>> I noticed there are a couple of dump_printf_loc instances in >>>> coverage.c not ended with newline. They should be fixed. >>> >>> I committed this change this morning as r202628. I believe I fixed all >>> the dump_printf_loc calls (just double-checked). Can you let me know >>> if you see anymore after you update to this revision? >>> >>> Thanks, >>> Teresa >>> >>>> >>>> David >>>> >>>> On Tue, Sep 10, 2013 at 6:32 AM, Teresa Johnson <tejohnson@google.com> wrote: >>>>> On Mon, Sep 9, 2013 at 9:55 PM, Xinliang David Li <davidxl@google.com> wrote: >>>>>> looks fine to me. >>>>>> >>>>>> In the long run, I wonder if the machinery in diagnostic messages can >>>>>> be reused for opt-info dumping -- i.e., support different streams. It >>>>>> has many nice features including %qD specifier for printing tree >>>>>> decls. >>>>> >>>>> Yes, this would have some advantages such as getting the function name emitted. >>>>> >>>>> Teresa >>>>> >>>>>> >>>>>> David >>>>>> >>>>>> On Mon, Sep 9, 2013 at 12:01 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>>>>> I've attached a patch that implements the cleanup of newline emission >>>>>>> by the new dump framework as discussed here: >>>>>>> >>>>>>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01779.html >>>>>>> >>>>>>> Essentially, I have removed the leading newline emission from >>>>>>> dump_loc, and updated dump_printf_loc invocations to emit a trailing >>>>>>> newline as necessary. This will remove unnecessary vertical space in >>>>>>> the dump output. >>>>>>> >>>>>>> I did not do any other cleanup of the existing vectorization messages >>>>>>> - there are IMO a lot of messages being emitted by the vectorizer >>>>>>> under MSG_NOTE (and probably MSG_MISSED_OPTIMIZATION) that should only >>>>>>> be emitted to the dump file under -fdump-tree-... and not emitted >>>>>>> under -fopt-info-all. The ones that stay under -fopt-info-all need >>>>>>> some formatting/style cleanup. Leaving that for follow-on work. >>>>>>> >>>>>>> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk? >>>>>>> >>>>>>> Thanks, >>>>>>> Teresa >>>>>>> >>>>>>> -- >>>>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>>>> >>>>> >>>>> >>>>> -- >>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Index: coverage.c =================================================================== --- coverage.c (revision 202628) +++ coverage.c (working copy) @@ -347,7 +347,7 @@ get_coverage_counts (unsigned counter, unsigned ex if (!warned++ && dump_enabled_p ()) dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location, (flag_guess_branch_prob - ? "file %s not found, execution counts estimated" + ? "file %s not found, execution counts estimated\n" : "file %s not found, execution counts assumed to " "be zero\n"), da_file_name);
Yep, looked too quickly every time and thought the newline after "be zero" was applying. Here is the patch with the fix. Ok for trunk pending regression testing? 2013-09-16 Teresa Johnson <tejohnson@google.com> * coverage.c (get_coverage_counts): Add missing newline. Thanks, Teresa On Mon, Sep 16, 2013 at 11:20 AM, Xinliang David Li <davidxl@google.com> wrote: > Looks like there is one missing spot: > > @@ -349,7 +349,7 @@ get_coverage_counts (unsigned counter, u > (flag_guess_branch_prob > ? "file %s not found, execution counts > estimated" <---- > : "file %s not found, execution counts assumed to " > - "be zero"), > + "be zero\n"), > da_file_name); > return NULL; > > > I found this when testing interaction of -fprofile-use and > -fno-tree-vectorize without a profile. > > thanks, > > David > > > On Mon, Sep 16, 2013 at 11:06 AM, Teresa Johnson <tejohnson@google.com> wrote: >> On Mon, Sep 16, 2013 at 10:57 AM, Xinliang David Li <davidxl@google.com> wrote: >>> I noticed there are a couple of dump_printf_loc instances in >>> coverage.c not ended with newline. They should be fixed. >> >> I committed this change this morning as r202628. I believe I fixed all >> the dump_printf_loc calls (just double-checked). Can you let me know >> if you see anymore after you update to this revision? >> >> Thanks, >> Teresa >> >>> >>> David >>> >>> On Tue, Sep 10, 2013 at 6:32 AM, Teresa Johnson <tejohnson@google.com> wrote: >>>> On Mon, Sep 9, 2013 at 9:55 PM, Xinliang David Li <davidxl@google.com> wrote: >>>>> looks fine to me. >>>>> >>>>> In the long run, I wonder if the machinery in diagnostic messages can >>>>> be reused for opt-info dumping -- i.e., support different streams. It >>>>> has many nice features including %qD specifier for printing tree >>>>> decls. >>>> >>>> Yes, this would have some advantages such as getting the function name emitted. >>>> >>>> Teresa >>>> >>>>> >>>>> David >>>>> >>>>> On Mon, Sep 9, 2013 at 12:01 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>>>> I've attached a patch that implements the cleanup of newline emission >>>>>> by the new dump framework as discussed here: >>>>>> >>>>>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01779.html >>>>>> >>>>>> Essentially, I have removed the leading newline emission from >>>>>> dump_loc, and updated dump_printf_loc invocations to emit a trailing >>>>>> newline as necessary. This will remove unnecessary vertical space in >>>>>> the dump output. >>>>>> >>>>>> I did not do any other cleanup of the existing vectorization messages >>>>>> - there are IMO a lot of messages being emitted by the vectorizer >>>>>> under MSG_NOTE (and probably MSG_MISSED_OPTIMIZATION) that should only >>>>>> be emitted to the dump file under -fdump-tree-... and not emitted >>>>>> under -fopt-info-all. The ones that stay under -fopt-info-all need >>>>>> some formatting/style cleanup. Leaving that for follow-on work. >>>>>> >>>>>> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk? >>>>>> >>>>>> Thanks, >>>>>> Teresa >>>>>> >>>>>> -- >>>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >>>> >>>> >>>> >>>> -- >>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413