This is the revised patch as suggested. How does it look? Thanks, David On Fri, Jun 10, 2011 at 9:22 AM, Xinliang David Li wrote: > On Fri, Jun 10, 2011 at 1:52 AM, Richard Guenther > wrote: >> On Thu, Jun 9, 2011 at 5:47 PM, Xinliang David Li 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 >>> wrote: >>>> On Thu, Jun 9, 2011 at 12:31 AM, Xinliang David Li 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 wrote: >>>>>> On Wed, Jun 8, 2011 at 2:06 AM, Richard Guenther >>>>>> wrote: >>>>>>> On Wed, Jun 8, 2011 at 1:08 AM, Xinliang David Li 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 >>>>>>>> wrote: >>>>>>>>> On Mon, Jun 6, 2011 at 6:20 PM, Xinliang David Li 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. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >