Re: Dump before flag

[Xinliang David Li <davidxl@google.com>]

On Wed, Jun 8, 2011 at 2:06 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> 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?

Yes.

>"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?

But the key usefulness here is pass isolation -- you don't need to
know the phase ordering -- the 'before' and 'after' are relative to
the pass that is specified.

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

The only downside is it loses fine grain control --- e.g, split IR
emission for the any one of the dumping points (before start todo,
before pass, after pass, after finish todo) into a separate file when
that point is specified. The content of the default dumping remains
unchanged  unless the 'finish' point is specified explicitly. In
short, the patch I had has minimal impact on default behavior, just
adding the capability to extract clean IR into different files for
diffing purpose.


> 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 don't quite understand this comment.  The processing order of
functions are kept in the split files.The main purpose of splitting is
to ease diffing for any transformation -- and our current dump can be
really hard to parse :)

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

Yes -- that can be done.

Thanks,

David

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