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

[Xinliang David Li <davidxl at google dot com>]

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