Dump before flag

Xinliang David Li davidxl@google.com
Tue Jun 14 23:22:00 GMT 2011


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 <davidxl@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 <davidxl@google.com> wrote:
>> On Tue, Jun 14, 2011 at 6:58 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Fri, Jun 10, 2011 at 8:44 PM, Xinliang David Li <davidxl@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 <davidxl@google.com> wrote:
>>>>> On Fri, Jun 10, 2011 at 1:52 AM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Thu, Jun 9, 2011 at 5:47 PM, Xinliang David Li <davidxl@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.guenther@gmail.com> wrote:
>>>>>>>> On Thu, Jun 9, 2011 at 12:31 AM, Xinliang David Li <davidxl@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 <davidxl@google.com> wrote:
>>>>>>>>>> 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?
>>>>>>>>>>
>>>>>>>>>> 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.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.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dump-where.p
Type: text/x-pascal
Size: 8212 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110614/d015711e/attachment.bin>


More information about the Gcc-patches mailing list