Attached the patch. David
On Tue, Jun 14, 2011 at 4:21 PM, Xinliang David Li <davi...@google.com> wrote: > This is the second (hopefully the last in the series of dumper > changes) follow-up patch. > > It adds a control so that verbosity of the dump can be greatly reduced > (and hopefully simplify gcc developer's life a little). For instance: > > -fdump-tree-dce="foo[0-9]$" > > to dump IR (and debug trace) only for function with assembler names > matching the specified pattern. > > Notes: > > 1) the pattern is specified using regular expression -- unlike > disable/enable option where exact name patterns are required, for > dumping, fuzziness is preferred > 2) patterns specified in different -fdump-.. options will be 'ORed' together. > > The patch has not been fully tested, but comments are welcome. > > Thanks, > > David > > > > On Tue, Jun 14, 2011 at 4:13 PM, Xinliang David Li <davi...@google.com> wrote: >> Here is one the of follow up patches: support of -before_preparation, >> -before, -after, -after_cleanup dump flags. >> >> The default dumping behavior does not change at all, but if any one of >> the above flags is specified, the function IR will be dumped into a >> file with the corresponding suffix. The enhancement is to simplify IR >> diffing. >> >> Bootstrapped and regression tested on x86-64/linux. >> >> Ok for trunk? >> >> Thanks, >> >> David >> >> On Tue, Jun 14, 2011 at 12:40 PM, Xinliang David Li <davi...@google.com> >> wrote: >>> Committed after Bootstrapping and regression testing on x86-64/linux. >>> The follow up patch will come soon. >>> >>> Thanks, >>> >>> David >>> >>> On Tue, Jun 14, 2011 at 8:57 AM, Xinliang David Li <davi...@google.com> >>> wrote: >>>> On Tue, Jun 14, 2011 at 6:58 AM, Richard Guenther >>>> <richard.guent...@gmail.com> wrote: >>>>> On Fri, Jun 10, 2011 at 8:44 PM, Xinliang David Li <davi...@google.com> >>>>> wrote: >>>>>> This is the revised patch as suggested. >>>>>> >>>>>> How does it look? >>>>> >>>>> } >>>>> >>>>> +static void >>>>> +execute_function_dump (void *data ATTRIBUTE_UNUSED) >>>>> >>>>> function needs a comment. >>>>> >>>>> Ok with that change. >>>>> >>>>> Please always specify how you tested the patch - the past fallouts >>>>> suggest you didn't do the required testing carefully. >>>> >>>> I think I did -- the fallout was probably due to different >>>> '--enable-checking' setting. I have now turned it to 'yes' >>>> >>>> Thanks, >>>> >>>> David >>>> >>>>> >>>>> A changelog is missing as well. >>>>> >>>>> Thanks, >>>>> Richard. >>>>> >>>>>> Thanks, >>>>>> >>>>>> David >>>>>> >>>>>> On Fri, Jun 10, 2011 at 9:22 AM, Xinliang David Li <davi...@google.com> >>>>>> wrote: >>>>>>> On Fri, Jun 10, 2011 at 1:52 AM, Richard Guenther >>>>>>> <richard.guent...@gmail.com> wrote: >>>>>>>> On Thu, Jun 9, 2011 at 5:47 PM, Xinliang David Li <davi...@google.com> >>>>>>>> wrote: >>>>>>>>> See attached. >>>>>>>> >>>>>>>> Hmm. I don't like how you still wire dumping in the TODO routines. >>>>>>>> Doesn't it work to just dump the body from pass_fini_dump_file ()? >>>>>>>> Or if that doesn't sound clean from (a subset of) places where it >>>>>>>> is called? (we might want to exclude the ipa read/write/summary >>>>>>>> stages) >>>>>>> >>>>>>> That may require another round of function traversal -- but probably >>>>>>> not a big deal -- it sounds cleaner. >>>>>>> >>>>>>> David >>>>>>> >>>>>>>> >>>>>>>> Richard. >>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> >>>>>>>>> David >>>>>>>>> >>>>>>>>> On Thu, Jun 9, 2011 at 2:02 AM, Richard Guenther >>>>>>>>> <richard.guent...@gmail.com> wrote: >>>>>>>>>> On Thu, Jun 9, 2011 at 12:31 AM, Xinliang David Li >>>>>>>>>> <davi...@google.com> wrote: >>>>>>>>>>> this is the patch that just removes the TODO_dump flag and forces it >>>>>>>>>>> to dump. The original code cfun->last_verified = flags & >>>>>>>>>>> TODO_verify_all looks weird -- depending on TODO_dump is set or not, >>>>>>>>>>> the behavior of the update is different (when no other todo flags is >>>>>>>>>>> set). >>>>>>>>>>> >>>>>>>>>>> Ok for trunk? >>>>>>>>>> >>>>>>>>>> -ENOPATCH. >>>>>>>>>> >>>>>>>>>> Richard. >>>>>>>>>> >>>>>>>>>>> David >>>>>>>>>>> >>>>>>>>>>> On Wed, Jun 8, 2011 at 9:52 AM, Xinliang David Li >>>>>>>>>>> <davi...@google.com> wrote: >>>>>>>>>>>> On Wed, Jun 8, 2011 at 2:06 AM, Richard Guenther >>>>>>>>>>>> <richard.guent...@gmail.com> wrote: >>>>>>>>>>>>> On Wed, Jun 8, 2011 at 1:08 AM, Xinliang David Li >>>>>>>>>>>>> <davi...@google.com> wrote: >>>>>>>>>>>>>> The following is the patch that does the job. Most of the >>>>>>>>>>>>>> changes are >>>>>>>>>>>>>> just removing TODO_dump_func. The major change is in passes.c >>>>>>>>>>>>>> and >>>>>>>>>>>>>> tree-pass.h. >>>>>>>>>>>>>> >>>>>>>>>>>>>> -fdump-xxx-yyy-start <-- dump before TODO_start >>>>>>>>>>>>>> -fdump-xxx-yyy-before <-- dump before main pass after >>>>>>>>>>>>>> TODO_pass >>>>>>>>>>>>>> -fdump-xxx-yyy-after <-- dump after main pass before >>>>>>>>>>>>>> TODO_finish >>>>>>>>>>>>>> -fdump-xxx-yyy-finish <-- dump after TODO_finish >>>>>>>>>>>>> >>>>>>>>>>>>> Can we bikeshed a bit more about these names? >>>>>>>>>>>> >>>>>>>>>>>> These names may be less confusing: >>>>>>>>>>>> >>>>>>>>>>>> before_preparation >>>>>>>>>>>> before >>>>>>>>>>>> after >>>>>>>>>>>> after_cleanup >>>>>>>>>>>> >>>>>>>>>>>> David >>>>>>>>>>>> >>>>>>>>>>>>> "start" and "before" >>>>>>>>>>>>> have no semantical difference to me ... as the dump before >>>>>>>>>>>>> TODO_start >>>>>>>>>>>>> of a pass and the dump after TODO_finish of the previous pass are >>>>>>>>>>>>> identical (hopefully ;)), maybe merge those into a -between flag? >>>>>>>>>>>>> If you'd specify it for a single pass then you'd get both -start >>>>>>>>>>>>> and -finish >>>>>>>>>>>>> (using your naming scheme). Splitting that dump(s) to different >>>>>>>>>>>>> files >>>>>>>>>>>>> then might make sense (not sure about the name to use). >>>>>>>>>>>>> >>>>>>>>>>>>> Note that I find it extremely useful to have dumping done in >>>>>>>>>>>>> chronological order - splitting some of it to different files >>>>>>>>>>>>> destroys >>>>>>>>>>>>> this, especially a dump after TODO_start or before TODO_finish >>>>>>>>>>>>> should appear in the same file (or we could also start splitting >>>>>>>>>>>>> individual TODO_ output into sub-dump-files). I guess what would >>>>>>>>>>>>> be nice instread would be a fancy dump-file viewer that could >>>>>>>>>>>>> show diffs, hide things like SCEV output, etc. >>>>>>>>>>>>> >>>>>>>>>>>>> I suppose a patch that removes the dump TODO and unconditionally >>>>>>>>>>>>> dumps at the current point would be a good preparation for this >>>>>>>>>>>>> enhancing patch. >>>>>>>>>>>>> >>>>>>>>>>>>> Richard. >>>>>>>>>>>>> >>>>>>>>>>>>>> The default is 'finish'. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Does it look ok? >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>> >>>>>>>>>>>>>> David >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Tue, Jun 7, 2011 at 2:36 AM, Richard Guenther >>>>>>>>>>>>>> <richard.guent...@gmail.com> wrote: >>>>>>>>>>>>>>> On Mon, Jun 6, 2011 at 6:20 PM, Xinliang David Li >>>>>>>>>>>>>>> <davi...@google.com> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Your patch doesn't really improve this but adds to the >>>>>>>>>>>>>>>>> confusion. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> + /* Override dump TODOs. */ >>>>>>>>>>>>>>>>> + if (dump_file && (pass->todo_flags_finish & TODO_dump_func) >>>>>>>>>>>>>>>>> + && (dump_flags & TDF_BEFORE)) >>>>>>>>>>>>>>>>> + { >>>>>>>>>>>>>>>>> + pass->todo_flags_finish &= ~TODO_dump_func; >>>>>>>>>>>>>>>>> + pass->todo_flags_start |= TODO_dump_func; >>>>>>>>>>>>>>>>> + } >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> and certainly writing to pass is not ok. And the TDF_BEFORE >>>>>>>>>>>>>>>>> flag >>>>>>>>>>>>>>>>> looks misplaced as it controls TODOs, not dumping behavior. >>>>>>>>>>>>>>>>> Yes, it's a mess right now but the above looks like a hack >>>>>>>>>>>>>>>>> ontop >>>>>>>>>>>>>>>>> of that mess (maybe because of it, but well ...). >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> How about removing dumping TODO completely -- this can be done >>>>>>>>>>>>>>>> easily >>>>>>>>>>>>>>>> -- I don't understand why pass wants extra control on the >>>>>>>>>>>>>>>> dumping if >>>>>>>>>>>>>>>> user already asked for dumping -- it is annoying to see empty >>>>>>>>>>>>>>>> IR dump >>>>>>>>>>>>>>>> for a pass when I want to see it. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> At least I would have expected to also get the dump after the >>>>>>>>>>>>>>>>> pass, not only the one before it with this dump flag. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Now, why can't you look at the previous pass output for the >>>>>>>>>>>>>>>>> before-dump (as I do usually)? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> For one thing, you need to either remember what is the >>>>>>>>>>>>>>>> previous pass, >>>>>>>>>>>>>>>> or dump all passes which for large files can take very long >>>>>>>>>>>>>>>> time. Even >>>>>>>>>>>>>>>> with all the dumps, you will need to eyeballing to find the >>>>>>>>>>>>>>>> previous >>>>>>>>>>>>>>>> pass which may or may not have the IR dumped. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> How about removing dump TODO? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Yeah, I think this would go in the right direction. Currently >>>>>>>>>>>>>>> some passes >>>>>>>>>>>>>>> do not dump function bodies because they presumably do no IL >>>>>>>>>>>>>>> modification. But this is certainly the minority (and some >>>>>>>>>>>>>>> passes do not >>>>>>>>>>>>>>> dump bodies even though they are modifying the IL ...). >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> So I'd say we should by default dump function bodies. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Note that there are three useful dumping positions (maybe four), >>>>>>>>>>>>>>> before todo-start, after todo-start, before todo-finish and >>>>>>>>>>>>>>> after todo-finish. >>>>>>>>>>>>>>> By default we'd want after todo-finish. When we no longer dump >>>>>>>>>>>>>>> via >>>>>>>>>>>>>>> a TODO then we could indeed use dump-flags to control this >>>>>>>>>>>>>>> (maybe -original for the body before todo-start). >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> What to others think? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Richard. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> David >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Richard. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
Index: tree-dump.c =================================================================== --- tree-dump.c (revision 174929) +++ tree-dump.c (working copy) @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. #include "tree-pass.h" #include "langhooks.h" #include "tree-iterator.h" +#include "xregex.h" /* If non-NULL, return one past-the-end of the matching SUBPART of the WHOLE string. */ @@ -909,6 +910,42 @@ get_dump_file_name (int phase) return concat (dump_base_name, dump_id, dfi->suffix, NULL); } +struct reg_func_patterns +{ + regex_t r; + struct reg_func_patterns *next; +} *reg_func_patterns; + +/* Retrurns true if dumping is enabled for FUNC. */ + +static bool +is_dump_enabled_for_func (tree func) +{ + const char *aname; + struct reg_func_patterns *one_pat; + + if (!func) + return true; + + one_pat = reg_func_patterns; + if (!one_pat) + return true; + + if (!DECL_ASSEMBLER_NAME_SET_P (func)) + return true; + + aname = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (func)); + do + { + if (regexec (&one_pat->r, aname, 0, NULL, 0) == 0) + return true; + one_pat = one_pat->next; + } while (one_pat); + + return false; +} + + /* 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. @@ -924,6 +961,9 @@ dump_begin (int phase, int *flag_ptr) if (phase == TDI_none || !dump_enabled_p (phase)) return NULL; + if (!is_dump_enabled_for_func (current_function_decl)) + return NULL; + name = get_dump_file_name (phase); dfi = get_dump_file_info (phase); stream = fopen (name, dfi->state < 0 ? "w" : "a"); @@ -1083,23 +1123,45 @@ dump_switch_p (const char *arg) { size_t i; int any = 0; + char *arg_dup, *pattern_str; + + arg_dup = xstrdup (arg); + pattern_str = strchr (arg_dup, '='); + if (pattern_str) + { + struct reg_func_patterns *one_pat = XCNEW (struct reg_func_patterns); + int ec; + + *pattern_str = '\0'; + pattern_str += 1; + + one_pat->next = reg_func_patterns; + reg_func_patterns = one_pat; + if ((ec= regcomp (&one_pat->r, pattern_str, REG_EXTENDED|REG_NOSUB) != 0)) + { + char err[100]; + regerror (ec, &one_pat->r, err, 99); + error ("invalid pattern specified in -fdump- option: %qs: %qs", pattern_str, err); + } + } for (i = TDI_none + 1; i != TDI_end; i++) - any |= dump_switch_p_1 (arg, &dump_files[i], false); + any |= dump_switch_p_1 (arg_dup, &dump_files[i], false); /* Don't glob if we got a hit already */ if (!any) for (i = TDI_none + 1; i != TDI_end; i++) - any |= dump_switch_p_1 (arg, &dump_files[i], true); + any |= dump_switch_p_1 (arg_dup, &dump_files[i], true); for (i = 0; i < extra_dump_files_in_use; i++) - any |= dump_switch_p_1 (arg, &extra_dump_files[i], false); + any |= dump_switch_p_1 (arg_dup, &extra_dump_files[i], false); if (!any) for (i = 0; i < extra_dump_files_in_use; i++) - any |= dump_switch_p_1 (arg, &extra_dump_files[i], true); + any |= dump_switch_p_1 (arg_dup, &extra_dump_files[i], true); + free (arg_dup); return any; }