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

Richard Guenther richard.guenther@gmail.com
Sat May 12 10:31:00 GMT 2012


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.

> 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



More information about the Gcc-patches mailing list