Attached the patch.
David
On Tue, Jun 14, 2011 at 4:21 PM, Xinliang David Li <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> 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 <[email protected]>
>>> wrote:
>>>> On Tue, Jun 14, 2011 at 6:58 AM, Richard Guenther
>>>> <[email protected]> wrote:
>>>>> On Fri, Jun 10, 2011 at 8:44 PM, Xinliang David Li <[email protected]>
>>>>> 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 <[email protected]>
>>>>>> wrote:
>>>>>>> On Fri, Jun 10, 2011 at 1:52 AM, Richard Guenther
>>>>>>> <[email protected]> wrote:
>>>>>>>> On Thu, Jun 9, 2011 at 5:47 PM, Xinliang David Li <[email protected]>
>>>>>>>> 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
>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>> On Thu, Jun 9, 2011 at 12:31 AM, Xinliang David Li
>>>>>>>>>> <[email protected]> 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
>>>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>>>> On Wed, Jun 8, 2011 at 2:06 AM, Richard Guenther
>>>>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>>>>> On Wed, Jun 8, 2011 at 1:08 AM, Xinliang David Li
>>>>>>>>>>>>> <[email protected]> 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
>>>>>>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>>>>>>> On Mon, Jun 6, 2011 at 6:20 PM, Xinliang David Li
>>>>>>>>>>>>>>> <[email protected]> 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;
}
