This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Dump before flag
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.
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.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>