Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1637)

Issue 6190057: [PATCH] Add option for dumping to stderr

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 2 months ago by Sharad Singhai
Modified:
9 years, 10 months ago
Reviewers:
richard.guenther, pinskia
CC:
davidxl, gcc-patches_gcc.gnu.org
Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/trunk/gcc/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : [PATCH] Add option for dumping to stderr #

Patch Set 3 : [PATCH] Add option for dumping to stderr #

Patch Set 4 : Updated patch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+200 lines, -58 lines) Patch
M doc/invoke.texi View 1 2 3 4 chunks +63 lines, -16 lines 0 comments Download
A testsuite/g++.dg/other/dump-filename-1.C View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M tree-dump.c View 1 2 3 9 chunks +122 lines, -41 lines 0 comments Download
M tree-pass.h View 1 2 2 chunks +4 lines, -1 line 0 comments Download

Messages

Total messages: 13
Sharad Singhai
This is the first patch for planned improvements to dump infrastructure. Please reference the discussion ...
12 years, 2 months ago (2012-05-07 21:58:46 UTC) #1
Sharad Singhai
In response to comments, I have updated the patch to support dumps in user provided ...
12 years, 2 months ago (2012-05-09 06:46:38 UTC) #2
pinskia_gmail.com
On Tue, May 8, 2012 at 11:46 PM, Sharad Singhai <singhai@google.com> wrote: > In response ...
12 years, 2 months ago (2012-05-09 06:51:58 UTC) #3
Sharad Singhai
Thanks for your suggestions/comments. I have updated the patch and documentation. It supports the following ...
12 years, 2 months ago (2012-05-09 23:57:40 UTC) #4
davidxl
Bummer. I was thinking to reserve '=' for selective dumping: -fdump-tree-pre=<func_list_regexp> I guess this can ...
12 years, 2 months ago (2012-05-10 00:31:17 UTC) #5
richard.guenther_gmail.com
On Thu, May 10, 2012 at 2:31 AM, Xinliang David Li <davidxl@google.com> wrote: > Bummer. ...
12 years, 1 month ago (2012-05-10 08:18:08 UTC) #6
Sharad Singhai
Okay, I have restored the original behavior where standard streams were considered weak. Thus in ...
12 years, 1 month ago (2012-05-10 08:19:25 UTC) #7
davidxl
I like your suggestion and support the end goal you have. I don't like the ...
12 years, 1 month ago (2012-05-10 16:28:24 UTC) #8
richard.guenther_gmail.com
On Thu, May 10, 2012 at 6:28 PM, Xinliang David Li <davidxl@google.com> wrote: > I ...
12 years, 1 month ago (2012-05-11 08:49:01 UTC) #9
davidxl
On Fri, May 11, 2012 at 1:49 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Thu, ...
12 years, 1 month ago (2012-05-11 16:06:47 UTC) #10
davidxl
To be more specific, does the following match what your envisioned? 1) when multiple streams ...
12 years, 1 month ago (2012-05-11 18:11:42 UTC) #11
richard.guenther_gmail.com
On Fri, May 11, 2012 at 8:11 PM, Xinliang David Li <davidxl@google.com> wrote: > To ...
12 years, 1 month ago (2012-05-12 10:31:16 UTC) #12
davidxl
12 years, 1 month ago (2012-05-12 16:05:49 UTC) #13
Sounds good.

On Sat, May 12, 2012 at 3:31 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Fri, May 11, 2012 at 8:11 PM, Xinliang David Li <davidxl@google.com>
wrote:
>> To be more specific, does the following match what your envisioned?
>>
>> 1) when multiple streams are specified for dumping, the information
>> will be dumped to all streams IF the new dumping interfaces are used
>> (see below). For legacy code, the default dump file will still be used
>> and the user specified file (including stderr) will be silently
>> ignored. This is of course temporary until all dumping sites are
>> converted
>
> Yes, that sounds reasonable.
>
>> 2) Define the following new dumping interfaces which will honor all
>> streams specified
>>
>>  dump_printf (const char*, ....);   // for raw string dumping
>>  dump_generic_expr (...); //  wrapper to print_generic_expr without
>> taking FILE*
>>  dump_node (...); // wrapper to print_node
>>  dump_gimple_stmt(...); // wrapper to print_gimple_stmt
>
> Yes.  Please add a classification argument to each of the wrappers
> to allow selective filtering (I'd simply use the existing TDF_* flags
> for now - in the future they should be made more granular than
> 0 vs. TDF_details).
>
> The existing if (dump_file && (dump_flags & ...)) checks need to be
> adjusted, of course.  if (dump_enabled_p ()) or if (dump_enabled_p (flags)).
>
>>  dump_inform (...); //wrapper to inform (..) method, but is aware of
>> of the dumping streams and modify global_dc appropriately.
>
> inform () is not part of the dumping infrastructure, thus passes should not
> use it.  The dumping implementation, if re-directed via -fopt-report, might
> use inform () to print messages though.
>

The downside is that the dump file format will look different from the
stderr output which is less than ideal. Setting and restore
diagnostic_context's printter stream is just one-line change, what is
the main concern of doing that?

thanks,

David


>> 3) Implement -fopt-info=N, and take -ftree-vectorizer-verbose as the
>> first guinea pig to use the new dumping interfaces.
>
> Yes, I think all of 1) to 2) do not make sense without 3), so I'd prefer
> to see all that (well, the relevant bits of 2) to make 3) work) together.
>
>> After the infrastructure is ready, gradually deprecate the use of the
>> original dumper interfaces.
>
> Yes, passes can be converted independently.
>
>> what do you think?
>
> That plan sounds good to me.
>
> Thanks,
> Richard.
>
>> thanks,
>>
>> David
>>
>> On Fri, May 11, 2012 at 9:06 AM, Xinliang David Li <davidxl@google.com>
wrote:
>>> On Fri, May 11, 2012 at 1:49 AM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Thu, May 10, 2012 at 6:28 PM, Xinliang David Li <davidxl@google.com>
wrote:
>>>>> I like your suggestion and support the end goal you have.  I don't
>>>>> like the -fopt-info behavior to interfere with regular -fdump-xxx
>>>>> options either.
>>>>>
>>>>> I think we should stage the changes in multiple steps as originally
>>>>> planned. Is Sharad's change good to be checked in for the first stage?
>>>>
>>>> Well - I don't think the change walks in the direction we want to go, so I
don't
>>>> see a good reason to make that change.
>>>
>>> Is your direction misunderstood or you want everything to be changed
>>> in one single patch (including replacement of all fprintf (dump_file,
>>> ...)? If the former, please clarify. If the latter, it can be done.
>>>
>>> thanks,
>>>
>>> David
>>>
>>>>
>>>>> After this one is checked in, the new dump interfaces will be worked
>>>>> on (and to allow multiple streams). Most of the remaining changes will
>>>>> be massive text replacement.
>>>>>
>>>>> thanks,
>>>>>
>>>>> David
>>>>>
>>>>>
>>>>> On Thu, May 10, 2012 at 1:18 AM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Thu, May 10, 2012 at 2:31 AM, Xinliang David Li <davidxl@google.com>
wrote:
>>>>>>> Bummer.  I was thinking to reserve '=' for selective  dumping:
>>>>>>>
>>>>>>> -fdump-tree-pre=<func_list_regexp>
>>>>>>>
>>>>>>> I guess this can be achieved via @
>>>>>>>
>>>>>>> -fdump-tree-pre@<func_list>
>>>>>>>
>>>>>>> -fdump-tree-pre=<file_name>@<func_list>
>>>>>>>
>>>>>>>
>>>>>>> Another issue -- I don't think the current precedence rule is correct.
>>>>>>> Consider that -fopt-info=2 will be mapped to
>>>>>>>
>>>>>>> -fdump-tree-all-transform-verbose2=stderr
>>>>>>> -fdump-rtl-all-transform-verbose2=stderr
>>>>>>>
>>>>>>> then
>>>>>>>
>>>>>>> the current precedence rule will cause surprise when the following is
used
>>>>>>>
>>>>>>> -fopt-info -fdump-tree-pre
>>>>>>>
>>>>>>> The PRE dump will be emitted to stderr which is not what user wants.
>>>>>>> In short, special streams should be treated as 'weak' the same way as
>>>>>>> your previous implementation.
>>>>>>
>>>>>> Hm, this raises a similar concern I have with the -fvectorizer-verbose
flag.
>>>>>> With -fopt-info -fdump-tree-pre I do not want some information to be
>>>>>> present only on stderr or in the dump file!  I want it in _both_ places!
>>>>>> (-fvectorizer-verbose makes the -fdump-tree-vect dump contain less
>>>>>> information :()
>>>>>>
>>>>>> Thus, the information where dumping goes has to be done differently
>>>>>> (which is why I asked for some re-org originally, so that passes no
>>>>>> longer explicitely reference dump_file - dump_file may be different
>>>>>> for different kind of information it dumps!).  Passes should, instead of
>>>>>>
>>>>>>  fprintf (dump_file, "...", ...)
>>>>>>
>>>>>> do
>>>>>>
>>>>>>  dump_printf (TDF_scev, "...", ...)
>>>>>>
>>>>>> thus, specify the kind of information they dump (would be mostly
>>>>>> TDF_details vs. 0 today I guess).  The dump_printf routine would
>>>>>> then properly direct to one or more places to dump at.
>>>>>>
>>>>>> I realize this needs some more dispatchers for dumping expressions
>>>>>> and statements (but it should not be too many).  Dumping to
>>>>>> dump_file would in any case dump to the passes private dump file
>>>>>> only (unqualified stuff would never be useful for -fopt-info).
>>>>>>
>>>>>> The perfect candidate to convert to this kind of scheme is obviously
>>>>>> the vectorizer with its existing -fvectorizer-verbose.
>>>>>>
>>>>>> If the patch doesn't work towards this kind of end-result I'd rather
>>>>>> not have it.
>>>>>>
>>>>>> Thanks,
>>>>>> Richard.
>>>>>>
>>>>>>> thanks,
>>>>>>>
>>>>>>> David
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, May 9, 2012 at 4:56 PM, Sharad Singhai <singhai@google.com>
wrote:
>>>>>>>> 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  <singhai@google.com>
>>>>>>>>
>>>>>>>>        * 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 streams. For
>>>>>>>> +example:
>>>>>>>> +
>>>>>>>> +@smallexample
>>>>>>>> +gcc -O2 -fdump-tree-pre=stderr -fdump-rtl-ira=ira.txt ...
>>>>>>>> +@end smallexample
>>>>>>>> +
>>>>>>>> +outputs PRE dump on @file{stderr}, while the IRA dump is output in a
>>>>>>>> +file named @file{ira.txt}.
>>>>>>>> +
>>>>>>>> +In case of any conflicts, the command line file name takes precedence
>>>>>>>> +over generated file names. For example:
>>>>>>>> +
>>>>>>>> +@smallexample
>>>>>>>> +gcc -O2 -fdump-tree-pre=stdout -fdump-tree-pre ...
>>>>>>>> +gcc -O2 -fdump-tree-pre -fdump-tree-pre=stdout ...
>>>>>>>> +@end smallexample
>>>>>>>> +
>>>>>>>> +Both of the above output the PRE dump on @file{stdout}. However, if
>>>>>>>> +there are multiple command line file names are applicable then the
>>>>>>>> +last one is used. Thus the command
>>>>>>>> +
>>>>>>>> +@smallexample
>>>>>>>> +gcc -O2 -fdump-tree-pre=stderr -fdump-tree-all=all.txt
>>>>>>>> +@end smallexample
>>>>>>>> +
>>>>>>>> +outputs all the dumps in @file{all.txt} because the stderr option for
>>>>>>>> +PRE dump is overridden by a later option.
>>>>>>>> +
>>>>>>>>  @item all
>>>>>>>> -Turn on all options, except @option{raw}, @option{slim},
@option{verbose}
>>>>>>>> -and @option{lineno}.
>>>>>>>> +Turn on all options, except @option{raw}, @option{slim},
@option{verbose},
>>>>>>>> +@option{lineno}.
>>>>>>>>  @end table
>>>>>>>>
>>>>>>>>  The following tree dumps are possible:
>>>>>>>> @@ -5913,6 +5956,7 @@ is made by appending @file{.vrp} to the source
fil
>>>>>>>>  @item all
>>>>>>>>  @opindex fdump-tree-all
>>>>>>>>  Enable all the available tree dumps with the flags provided in this
option.
>>>>>>>> +
>>>>>>>>  @end table
>>>>>>>>
>>>>>>>>  @item -ftree-vectorizer-verbose=@var{n}
>>>>>>>> Index: tree-dump.c
>>>>>>>> ===================================================================
>>>>>>>> --- tree-dump.c (revision 187265)
>>>>>>>> +++ tree-dump.c (working copy)
>>>>>>>> @@ -773,20 +773,20 @@ dump_node (const_tree t, int flags, FILE
*stream)
>>>>>>>>    tree_dump_index enumeration in tree-pass.h.  */
>>>>>>>>  static struct dump_file_info dump_files[TDI_end] =
>>>>>>>>  {
>>>>>>>> -  {NULL, NULL, NULL, 0, 0, 0},
>>>>>>>> -  {".cgraph", "ipa-cgraph", NULL, TDF_IPA, 0,  0},
>>>>>>>> -  {".tu", "translation-unit", NULL, TDF_TREE, 0, 1},
>>>>>>>> -  {".class", "class-hierarchy", NULL, TDF_TREE, 0, 2},
>>>>>>>> -  {".original", "tree-original", NULL, TDF_TREE, 0, 3},
>>>>>>>> -  {".gimple", "tree-gimple", NULL, TDF_TREE, 0, 4},
>>>>>>>> -  {".nested", "tree-nested", NULL, TDF_TREE, 0, 5},
>>>>>>>> -  {".vcg", "tree-vcg", NULL, TDF_TREE, 0, 6},
>>>>>>>> -  {".ads", "ada-spec", NULL, 0, 0, 7},
>>>>>>>> +  {NULL, NULL, NULL, NULL, 0, 0, 0},
>>>>>>>> +  {".cgraph", "ipa-cgraph", NULL, NULL, TDF_IPA, 0,  0},
>>>>>>>> +  {".tu", "translation-unit", NULL, NULL, TDF_TREE, 0, 1},
>>>>>>>> +  {".class", "class-hierarchy", NULL, NULL, TDF_TREE, 0, 2},
>>>>>>>> +  {".original", "tree-original", NULL, NULL, TDF_TREE, 0, 3},
>>>>>>>> +  {".gimple", "tree-gimple", NULL, NULL, TDF_TREE, 0, 4},
>>>>>>>> +  {".nested", "tree-nested", NULL, NULL, TDF_TREE, 0, 5},
>>>>>>>> +  {".vcg", "tree-vcg", NULL, NULL, TDF_TREE, 0, 6},
>>>>>>>> +  {".ads", "ada-spec", NULL, NULL, 0, 0, 7},
>>>>>>>>  #define FIRST_AUTO_NUMBERED_DUMP 8
>>>>>>>>
>>>>>>>> -  {NULL, "tree-all", NULL, TDF_TREE, 0, 0},
>>>>>>>> -  {NULL, "rtl-all", NULL, TDF_RTL, 0, 0},
>>>>>>>> -  {NULL, "ipa-all", NULL, TDF_IPA, 0, 0},
>>>>>>>> +  {NULL, "tree-all", NULL, NULL, TDF_TREE, 0, 0},
>>>>>>>> +  {NULL, "rtl-all", NULL, NULL, TDF_RTL, 0, 0},
>>>>>>>> +  {NULL, "ipa-all", NULL, NULL, TDF_IPA, 0, 0},
>>>>>>>>  };
>>>>>>>>
>>>>>>>>  /* Dynamically registered tree dump files and switches.  */
>>>>>>>> @@ -802,7 +802,7 @@ struct dump_option_value_info
>>>>>>>>  };
>>>>>>>>
>>>>>>>>  /* Table of dump options. This must be consistent with the TDF_*
flags
>>>>>>>> -   in tree.h */
>>>>>>>> +   in tree-pass.h */
>>>>>>>>  static const struct dump_option_value_info dump_options[] =
>>>>>>>>  {
>>>>>>>>   {"address", TDF_ADDRESS},
>>>>>>>> @@ -892,6 +892,9 @@ get_dump_file_name (int phase)
>>>>>>>>   if (dfi->state == 0)
>>>>>>>>     return NULL;
>>>>>>>>
>>>>>>>> +  if (dfi->filename)
>>>>>>>> +    return xstrdup (dfi->filename);
>>>>>>>> +
>>>>>>>>   if (dfi->num < 0)
>>>>>>>>     dump_id[0] = '\0';
>>>>>>>>   else
>>>>>>>> @@ -911,6 +914,22 @@ get_dump_file_name (int phase)
>>>>>>>>   return concat (dump_base_name, dump_id, dfi->suffix, NULL);
>>>>>>>>  }
>>>>>>>>
>>>>>>>> +/* If the DFI dump output corresponds to stdout or stderr stream,
>>>>>>>> +   return that stream, NULL otherwise.  */
>>>>>>>> +
>>>>>>>> +static FILE *
>>>>>>>> +dump_get_standard_stream (struct dump_file_info *dfi)
>>>>>>>> +{
>>>>>>>> +  if (!dfi->filename)
>>>>>>>> +    return NULL;
>>>>>>>> +
>>>>>>>> +  return strcmp("stderr", dfi->filename) == 0
>>>>>>>> +    ? stderr
>>>>>>>> +    : strcmp("stdout", dfi->filename) == 0
>>>>>>>> +    ?  stdout
>>>>>>>> +    : NULL;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>  /* Begin a tree dump for PHASE. Stores any user supplied flag in
>>>>>>>>    *FLAG_PTR and returns a stream to write to. If the dump is not
>>>>>>>>    enabled, returns NULL.
>>>>>>>> @@ -926,14 +945,20 @@ dump_begin (int phase, int *flag_ptr)
>>>>>>>>   if (phase == TDI_none || !dump_enabled_p (phase))
>>>>>>>>     return NULL;
>>>>>>>>
>>>>>>>> -  name = get_dump_file_name (phase);
>>>>>>>>   dfi = get_dump_file_info (phase);
>>>>>>>> -  stream = fopen (name, dfi->state < 0 ? "w" : "a");
>>>>>>>> -  if (!stream)
>>>>>>>> -    error ("could not open dump file %qs: %m", name);
>>>>>>>> +  stream = dump_get_standard_stream (dfi);
>>>>>>>> +  if (stream)
>>>>>>>> +    dfi->state = 1;
>>>>>>>>   else
>>>>>>>> -    dfi->state = 1;
>>>>>>>> -  free (name);
>>>>>>>> +    {
>>>>>>>> +      name = get_dump_file_name (phase);
>>>>>>>> +      stream = fopen (name, dfi->state < 0 ? "w" : "a");
>>>>>>>> +      if (!stream)
>>>>>>>> +        error ("could not open dump file %qs: %m", name);
>>>>>>>> +      else
>>>>>>>> +        dfi->state = 1;
>>>>>>>> +      free (name);
>>>>>>>> +    }
>>>>>>>>
>>>>>>>>   if (flag_ptr)
>>>>>>>>     *flag_ptr = dfi->flags;
>>>>>>>> @@ -987,35 +1012,46 @@ dump_flag_name (int phase)
>>>>>>>>    dump_begin.  */
>>>>>>>>
>>>>>>>>  void
>>>>>>>> -dump_end (int phase ATTRIBUTE_UNUSED, FILE *stream)
>>>>>>>> +dump_end (int phase, FILE *stream)
>>>>>>>>  {
>>>>>>>> -  fclose (stream);
>>>>>>>> +  struct dump_file_info *dfi = get_dump_file_info (phase);
>>>>>>>> +  if (!dump_get_standard_stream (dfi))
>>>>>>>> +    fclose (stream);
>>>>>>>>  }
>>>>>>>>
>>>>>>>> -/* Enable all tree dumps.  Return number of enabled tree dumps.  */
>>>>>>>> +/* Enable all tree dumps with FLAGS on FILENAME.  Return number of
>>>>>>>> +   enabled tree dumps.  */
>>>>>>>>
>>>>>>>>  static int
>>>>>>>> -dump_enable_all (int flags)
>>>>>>>> +dump_enable_all (int flags, const char *filename)
>>>>>>>>  {
>>>>>>>>   int ir_dump_type = (flags & (TDF_TREE | TDF_RTL | TDF_IPA));
>>>>>>>>   int n = 0;
>>>>>>>>   size_t i;
>>>>>>>>
>>>>>>>>   for (i = TDI_none + 1; i < (size_t) TDI_end; i++)
>>>>>>>> -    if ((dump_files[i].flags & ir_dump_type))
>>>>>>>> -      {
>>>>>>>> -        dump_files[i].state = -1;
>>>>>>>> -        dump_files[i].flags |= flags;
>>>>>>>> -        n++;
>>>>>>>> -      }
>>>>>>>> +    {
>>>>>>>> +      if ((dump_files[i].flags & ir_dump_type))
>>>>>>>> +        {
>>>>>>>> +          dump_files[i].state = -1;
>>>>>>>> +          dump_files[i].flags |= flags;
>>>>>>>> +          n++;
>>>>>>>> +          if (filename)
>>>>>>>> +            dump_files[i].filename = xstrdup (filename);
>>>>>>>> +        }
>>>>>>>> +    }
>>>>>>>>
>>>>>>>>   for (i = 0; i < extra_dump_files_in_use; i++)
>>>>>>>> -    if ((extra_dump_files[i].flags & ir_dump_type))
>>>>>>>> -      {
>>>>>>>> -        extra_dump_files[i].state = -1;
>>>>>>>> -        extra_dump_files[i].flags |= flags;
>>>>>>>> -       n++;
>>>>>>>> -      }
>>>>>>>> +    {
>>>>>>>> +      if ((extra_dump_files[i].flags & ir_dump_type))
>>>>>>>> +        {
>>>>>>>> +          extra_dump_files[i].state = -1;
>>>>>>>> +          extra_dump_files[i].flags |= flags;
>>>>>>>> +          n++;
>>>>>>>> +          if (filename)
>>>>>>>> +            extra_dump_files[i].filename = xstrdup (filename);
>>>>>>>> +        }
>>>>>>>> +    }
>>>>>>>>
>>>>>>>>   return n;
>>>>>>>>  }
>>>>>>>> @@ -1037,7 +1073,7 @@ dump_switch_p_1 (const char *arg, struct
dump_file
>>>>>>>>   if (!option_value)
>>>>>>>>     return 0;
>>>>>>>>
>>>>>>>> -  if (*option_value && *option_value != '-')
>>>>>>>> +  if (*option_value && *option_value != '-' && *option_value != '=')
>>>>>>>>     return 0;
>>>>>>>>
>>>>>>>>   ptr = option_value;
>>>>>>>> @@ -1052,17 +1088,30 @@ dump_switch_p_1 (const char *arg, struct
dump_file
>>>>>>>>       while (*ptr == '-')
>>>>>>>>        ptr++;
>>>>>>>>       end_ptr = strchr (ptr, '-');
>>>>>>>> +
>>>>>>>>       if (!end_ptr)
>>>>>>>>        end_ptr = ptr + strlen (ptr);
>>>>>>>>       length = end_ptr - ptr;
>>>>>>>>
>>>>>>>> +      if (*ptr == '=')
>>>>>>>> +        {
>>>>>>>> +          /* Interpret rest of the argument as a dump filename.  This
>>>>>>>> +             filename overrides generated dump names as well as other
>>>>>>>> +             command line filenames.  */
>>>>>>>> +          flags |= TDF_FILENAME;
>>>>>>>> +          if (dfi->filename)
>>>>>>>> +            free (dfi->filename);
>>>>>>>> +          dfi->filename = xstrdup (ptr + 1);
>>>>>>>> +          break;
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>>       for (option_ptr = dump_options; option_ptr->name; option_ptr++)
>>>>>>>>        if (strlen (option_ptr->name) == length
>>>>>>>>            && !memcmp (option_ptr->name, ptr, length))
>>>>>>>> -         {
>>>>>>>> -           flags |= option_ptr->value;
>>>>>>>> +          {
>>>>>>>> +            flags |= option_ptr->value;
>>>>>>>>            goto found;
>>>>>>>> -         }
>>>>>>>> +          }
>>>>>>>>       warning (0, "ignoring unknown option %q.*s in %<-fdump-%s%>",
>>>>>>>>               length, ptr, dfi->swtch);
>>>>>>>>     found:;
>>>>>>>> @@ -1075,7 +1124,7 @@ dump_switch_p_1 (const char *arg, struct
dump_file
>>>>>>>>   /* Process -fdump-tree-all and -fdump-rtl-all, by enabling all the
>>>>>>>>      known dumps.  */
>>>>>>>>   if (dfi->suffix == NULL)
>>>>>>>> -    dump_enable_all (dfi->flags);
>>>>>>>> +    dump_enable_all (dfi->flags, dfi->filename);
>>>>>>>>
>>>>>>>>   return 1;
>>>>>>>>  }
>>>>>>>> @@ -1124,5 +1173,5 @@ dump_function (int phase, tree fn)
>>>>>>>>  bool
>>>>>>>>  enable_rtl_dump_file (void)
>>>>>>>>  {
>>>>>>>> -  return dump_enable_all (TDF_RTL | TDF_DETAILS | TDF_BLOCKS) > 0;
>>>>>>>> +  return dump_enable_all (TDF_RTL | TDF_DETAILS | TDF_BLOCKS, NULL) >
0;
>>>>>>>>  }
>>>>>>>> Index: tree-pass.h
>>>>>>>> ===================================================================
>>>>>>>> --- tree-pass.h (revision 187265)
>>>>>>>> +++ tree-pass.h (working copy)
>>>>>>>> @@ -84,8 +84,9 @@ enum tree_dump_index
>>>>>>>>  #define TDF_ENUMERATE_LOCALS (1 << 22) /* Enumerate locals by uid.
 */
>>>>>>>>  #define TDF_CSELIB     (1 << 23)       /* Dump cselib details.  */
>>>>>>>>  #define TDF_SCEV       (1 << 24)       /* Dump SCEV details.  */
>>>>>>>> +#define TDF_FILENAME    (1 << 25)      /* Dump on provided filename
>>>>>>>> +                                           instead of constructing
one. */
>>>>>>>>
>>>>>>>> -
>>>>>>>>  /* In tree-dump.c */
>>>>>>>>
>>>>>>>>  extern char *get_dump_file_name (int);
>>>>>>>> @@ -222,6 +223,8 @@ struct dump_file_info
>>>>>>>>   const char *suffix;           /* suffix to give output file.  */
>>>>>>>>   const char *swtch;            /* command line switch */
>>>>>>>>   const char *glob;             /* command line glob  */
>>>>>>>> +  const char *filename;         /* use this filename instead of
making
>>>>>>>> +                                   up one using dump_base_name +
suffix.  */
>>>>>>>>   int flags;                    /* user flags */
>>>>>>>>   int state;                    /* state of play */
>>>>>>>>   int num;                      /* dump file number */
>>>>>>>> Index: testsuite/g++.dg/other/dump-filename-1.C
>>>>>>>> ===================================================================
>>>>>>>> --- testsuite/g++.dg/other/dump-filename-1.C    (revision 0)
>>>>>>>> +++ testsuite/g++.dg/other/dump-filename-1.C    (revision 0)
>>>>>>>> @@ -0,0 +1,11 @@
>>>>>>>> +// Test that the dump to a user-defined file works correctly.
>>>>>>>> +/* { dg-options "-O2 -fdump-tree-original=foobar.dump" } */
>>>>>>>> +
>>>>>>>> +void test (int *b, int *e, int stride)
>>>>>>>> +  {
>>>>>>>> +    for (int *p = b; p != e; p += stride)
>>>>>>>> +      *p = 1;
>>>>>>>> +  }
>>>>>>>> +
>>>>>>>> +/* { dg-final { scan-file foobar.dump ";; Function void test" } } */
>>>>>>>> +/* { dg-final { remove-build-file "foobar.dump" } } */
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, May 9, 2012 at 12:22 AM, Gabriel Dos Reis
>>>>>>>> <gdr@integrable-solutions.net> wrote:
>>>>>>>>> On Wed, May 9, 2012 at 1:46 AM, Sharad Singhai <singhai@google.com>
wrote:
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>> +@item -fdump-rtl-all=stderr
>>>>>>>>>> +@opindex fdump-rtl-all=stderr
>>>>>>>>>
>>>>>>>>> You do not need to have a separate index entry for '=stderr' or
'=stdout'.
>>>>>>>>> Rather, expand the description to state this in all the documentation
>>>>>>>>> for -fdump-xxx=yyy.
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>> +/* Return non-zero iff the USER_FILENAME corresponds to stdout or
>>>>>>>>>> +   stderr stream.  */
>>>>>>>>>> +
>>>>>>>>>> +static int
>>>>>>>>>> +dump_stream_p (const char *user_filename)
>>>>>>>>>> +{
>>>>>>>>>> +  if (user_filename)
>>>>>>>>>> +    return !strncmp ("stderr", user_filename, 6) ||
>>>>>>>>>> +      !strncmp ("stdout", user_filename, 6);
>>>>>>>>>> +  else
>>>>>>>>>> +    return 0;
>>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>> The name is ambiguous.
>>>>>>>>> This function is testing whether its string argument designates one
of
>>>>>>>>> the *standard* output streams.  Name it to reflect that..
>>>>>>>>> Have it take the dump state context. Also the coding convention: the
binary
>>>>>>>>> operator "||" should be on next line.  In fact the thing could be
>>>>>>>>> simpler.   Instead of
>>>>>>>>> testing over and over again against "stderr" (once in this function,
then again
>>>>>>>>> later), just return the corresponding standard FILE* pointer.
>>>>>>>>> Also, this is a case of overuse of strncmp.  If you name the function
>>>>>>>>> dump_get_standard_stream:
>>>>>>>>>
>>>>>>>>>    return strcmp("stderr", dfi->user_filename) == 0
>>>>>>>>>       ? stderr
>>>>>>>>>        : stdcmp("stdout", dfi->use_filename)
>>>>>>>>>        ?  stdout
>>>>>>>>>        : NULL;
>>>>>>>>>
>>>>>>>>> you can simplify:
>>>>>>>>>
>>>>>>>>>> -  name = get_dump_file_name (phase);
>>>>>>>>>>   dfi = get_dump_file_info (phase);
>>>>>>>>>> -  stream = fopen (name, dfi->state < 0 ? "w" : "a");
>>>>>>>>>> -  if (!stream)
>>>>>>>>>> -    error ("could not open dump file %qs: %m", name);
>>>>>>>>>> +  if (dump_stream_p (dfi->user_filename))
>>>>>>>>>> +    {
>>>>>>>>>> +      if (!strncmp ("stderr", dfi->user_filename, 6))
>>>>>>>>>> +        stream = stderr;
>>>>>>>>>> +      else
>>>>>>>>>> +        if (!strncmp ("stdout", dfi->user_filename, 6))
>>>>>>>>>> +          stream = stdout;
>>>>>>>>>> +        else
>>>>>>>>>> +          error ("unknown stream: %qs: %m", dfi->user_filename);
>>>>>>>>>> +      dfi->state = 1;
>>>>>>>>>> +    }
>>>>>>>>>>   else
>>>>>>>>>> -    dfi->state = 1;
>>>>>>>>>> -  free (name);
>>>>>>>>>> +    {
>>>>>>>>>> +      name = get_dump_file_name (phase);
>>>>>>>>>> +      stream = fopen (name, dfi->state < 0 ? "w" : "a");
>>>>>>>>>> +      if (!stream)
>>>>>>>>>> +        error ("could not open dump file %qs: %m", name);
>>>>>>>>>> +      else
>>>>>>>>>> +        dfi->state = 1;
>>>>>>>>>> +      free (name);
>>>>>>>>>> +    }
>>>>>>>>>>
>>>>>>>>>>   if (flag_ptr)
>>>>>>>>>>     *flag_ptr = dfi->flags;
>>>>>>>>>> @@ -987,35 +1017,45 @@ dump_flag_name (int phase)
>>>>>>>>>>    dump_begin.  */
>>>>>>>>>>
>>>>>>>>>>  void
>>>>>>>>>> -dump_end (int phase ATTRIBUTE_UNUSED, FILE *stream)
>>>>>>>>>> +dump_end (int phase, FILE *stream)
>>>>>>>>>>  {
>>>>>>>>>> -  fclose (stream);
>>>>>>>>>> +  struct dump_file_info *dfi = get_dump_file_info (phase);
>>>>>>>>>> +  if (!dump_stream_p (dfi->user_filename))
>>>>>>>>>> +    fclose (stream);
>>>>>>>>>>  }
>>>>>>>>>>
>>>>>>>>>>  /* Enable all tree dumps.  Return number of enabled tree dumps.  */
>>>>>>>>>>
>>>>>>>>>>  static int
>>>>>>>>>> -dump_enable_all (int flags)
>>>>>>>>>> +dump_enable_all (int flags, const char *user_filename)
>>>>>>>>>>  {
>>>>>>>>>>   int ir_dump_type = (flags & (TDF_TREE | TDF_RTL | TDF_IPA));
>>>>>>>>>>   int n = 0;
>>>>>>>>>>   size_t i;
>>>>>>>>>>
>>>>>>>>>>   for (i = TDI_none + 1; i < (size_t) TDI_end; i++)
>>>>>>>>>> -    if ((dump_files[i].flags & ir_dump_type))
>>>>>>>>>> -      {
>>>>>>>>>> -        dump_files[i].state = -1;
>>>>>>>>>> -        dump_files[i].flags |= flags;
>>>>>>>>>> -        n++;
>>>>>>>>>> -      }
>>>>>>>>>> +    {
>>>>>>>>>> +      if ((dump_files[i].flags & ir_dump_type))
>>>>>>>>>> +        {
>>>>>>>>>> +          dump_files[i].state = -1;
>>>>>>>>>> +          dump_files[i].flags |= flags;
>>>>>>>>>> +          n++;
>>>>>>>>>> +        }
>>>>>>>>>> +      if (user_filename)
>>>>>>>>>> +        dump_files[i].user_filename = user_filename;
>>>>>>>>>> +    }
>>>>>>>>>>
>>>>>>>>>>   for (i = 0; i < extra_dump_files_in_use; i++)
>>>>>>>>>> -    if ((extra_dump_files[i].flags & ir_dump_type))
>>>>>>>>>> -      {
>>>>>>>>>> -        extra_dump_files[i].state = -1;
>>>>>>>>>> -        extra_dump_files[i].flags |= flags;
>>>>>>>>>> -       n++;
>>>>>>>>>> -      }
>>>>>>>>>> +    {
>>>>>>>>>> +      if ((extra_dump_files[i].flags & ir_dump_type))
>>>>>>>>>> +        {
>>>>>>>>>> +          extra_dump_files[i].state = -1;
>>>>>>>>>> +          extra_dump_files[i].flags |= flags;
>>>>>>>>>> +          n++;
>>>>>>>>>> +        }
>>>>>>>>>> +      if (user_filename)
>>>>>>>>>> +        extra_dump_files[i].user_filename = user_filename;
>>>>>>>>>> +    }
>>>>>>>>>>
>>>>>>>>>>   return n;
>>>>>>>>>>  }
>>>>>>>>>> @@ -1037,7 +1077,7 @@ dump_switch_p_1 (const char *arg, struct
dump_file
>>>>>>>>>>   if (!option_value)
>>>>>>>>>>     return 0;
>>>>>>>>>>
>>>>>>>>>> -  if (*option_value && *option_value != '-')
>>>>>>>>>> +  if (*option_value && *option_value != '-' && *option_value !=
'=')
>>>>>>>>>>     return 0;
>>>>>>>>>>
>>>>>>>>>>   ptr = option_value;
>>>>>>>>>> @@ -1052,17 +1092,30 @@ dump_switch_p_1 (const char *arg, struct
dump_file
>>>>>>>>>>       while (*ptr == '-')
>>>>>>>>>>        ptr++;
>>>>>>>>>>       end_ptr = strchr (ptr, '-');
>>>>>>>>>> +
>>>>>>>>>>       if (!end_ptr)
>>>>>>>>>>        end_ptr = ptr + strlen (ptr);
>>>>>>>>>>       length = end_ptr - ptr;
>>>>>>>>>>
>>>>>>>>>> +      if (*ptr == '=')
>>>>>>>>>> +        {
>>>>>>>>>> +          /* Interpret rest of the argument as a dump filename.
 The
>>>>>>>>>> +             user provided filename overrides generated dump names
as
>>>>>>>>>> +             well as other command line filenames.  */
>>>>>>>>>> +          flags |= TDF_USER_FILENAME;
>>>>>>>>>> +          if (dfi->user_filename)
>>>>>>>>>> +            free (dfi->user_filename);
>>>>>>>>>> +          dfi->user_filename = xstrdup (ptr + 1);
>>>>>>>>>> +          break;
>>>>>>>>>> +        }
>>>>>>>>>> +
>>>>>>>>>>       for (option_ptr = dump_options; option_ptr->name;
option_ptr++)
>>>>>>>>>>        if (strlen (option_ptr->name) == length
>>>>>>>>>>            && !memcmp (option_ptr->name, ptr, length))
>>>>>>>>>> -         {
>>>>>>>>>> -           flags |= option_ptr->value;
>>>>>>>>>> +          {
>>>>>>>>>> +            flags |= option_ptr->value;
>>>>>>>>>>            goto found;
>>>>>>>>>> -         }
>>>>>>>>>> +          }
>>>>>>>>>>       warning (0, "ignoring unknown option %q.*s in %<-fdump-%s%>",
>>>>>>>>>>               length, ptr, dfi->swtch);
>>>>>>>>>>     found:;
>>>>>>>>>> @@ -1075,7 +1128,7 @@ dump_switch_p_1 (const char *arg, struct
dump_file
>>>>>>>>>>   /* Process -fdump-tree-all and -fdump-rtl-all, by enabling all the
>>>>>>>>>>      known dumps.  */
>>>>>>>>>>   if (dfi->suffix == NULL)
>>>>>>>>>> -    dump_enable_all (dfi->flags);
>>>>>>>>>> +    dump_enable_all (dfi->flags, dfi->user_filename);
>>>>>>>>>>
>>>>>>>>>>   return 1;
>>>>>>>>>>  }
>>>>>>>>>> @@ -1124,5 +1177,5 @@ dump_function (int phase, tree fn)
>>>>>>>>>>  bool
>>>>>>>>>>  enable_rtl_dump_file (void)
>>>>>>>>>>  {
>>>>>>>>>> -  return dump_enable_all (TDF_RTL | TDF_DETAILS | TDF_BLOCKS) > 0;
>>>>>>>>>> +  return dump_enable_all (TDF_RTL | TDF_DETAILS | TDF_BLOCKS, NULL)
> 0;
>>>>>>>>>>  }
>>>>>>>>>> Index: tree-pass.h
>>>>>>>>>> ===================================================================
>>>>>>>>>> --- tree-pass.h (revision 187265)
>>>>>>>>>> +++ tree-pass.h (working copy)
>>>>>>>>>> @@ -84,8 +84,9 @@ enum tree_dump_index
>>>>>>>>>>  #define TDF_ENUMERATE_LOCALS (1 << 22) /* Enumerate locals by uid.
 */
>>>>>>>>>>  #define TDF_CSELIB     (1 << 23)       /* Dump cselib details.  */
>>>>>>>>>>  #define TDF_SCEV       (1 << 24)       /* Dump SCEV details.  */
>>>>>>>>>> +#define TDF_USER_FILENAME    (1 << 25) /* Dump on user provided
filename,
>>>>>>>>>> +                                           instead of constructing
one. */
>>>>>>>>>>
>>>>>>>>>> -
>>>>>>>>>>  /* In tree-dump.c */
>>>>>>>>>>
>>>>>>>>>>  extern char *get_dump_file_name (int);
>>>>>>>>>> @@ -222,6 +223,8 @@ struct dump_file_info
>>>>>>>>>>   const char *suffix;           /* suffix to give output file.  */
>>>>>>>>>>   const char *swtch;            /* command line switch */
>>>>>>>>>>   const char *glob;             /* command line glob  */
>>>>>>>>>> +  const char *user_filename;    /* user provided filename instead of
making
>>>>>>>>>> +                                   up one using dump_base_name +
suffix.  */
>>>>>>>>>
>>>>>>>>> There is "no user" here: we are the users :-)  Just call it
"filename".
>>>>>>>>>
>>>>>>>>> -- Gaby
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b