On Thu, May 10, 2012 at 6:28 PM, Xinliang David Li <davi...@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.

> 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.guent...@gmail.com> wrote:
>> On Thu, May 10, 2012 at 2:31 AM, Xinliang David Li <davi...@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 <sing...@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  <sing...@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
>>>> <g...@integrable-solutions.net> wrote:
>>>>> On Wed, May 9, 2012 at 1:46 AM, Sharad Singhai <sing...@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

Reply via email to