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.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
