On Fri, May 11, 2012 at 1:49 AM, Richard Guenther <richard.guent...@gmail.com> wrote: > 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.
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.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