Re: Dump before flag

[Richard Guenther <richard.guenther@gmail.com>]

On Wed, Jun 8, 2011 at 1:08 AM, Xinliang David Li <davidxl@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?  "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.guenther@gmail.com> wrote:
>> On Mon, Jun 6, 2011 at 6:20 PM, Xinliang David Li <davidxl@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.
>>>>
>>>
>>
>