From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29657 invoked by alias); 14 Jun 2011 13:58:44 -0000 Received: (qmail 29643 invoked by uid 22791); 14 Jun 2011 13:58:41 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Jun 2011 13:58:22 +0000 Received: by wwf26 with SMTP id 26so5302919wwf.8 for ; Tue, 14 Jun 2011 06:58:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.163.76 with SMTP id z12mr6452091wbx.75.1308059900531; Tue, 14 Jun 2011 06:58:20 -0700 (PDT) Received: by 10.227.28.69 with HTTP; Tue, 14 Jun 2011 06:58:20 -0700 (PDT) In-Reply-To: References: <20110601231202.224188ad.basile@starynkevitch.net> Date: Tue, 14 Jun 2011 14:19:00 -0000 Message-ID: Subject: Re: Dump before flag From: Richard Guenther To: Xinliang David Li Cc: GCC Patches , Diego Novillo Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-06/txt/msg01052.txt.bz2 On Fri, Jun 10, 2011 at 8:44 PM, Xinliang David Li wro= te: > 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 w= rote: >> 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. =A0I 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 =3D 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 =A0removing TODO_dump_func. The major change is in passes.c = and >>>>>>>>> tree-pass.h. >>>>>>>>> >>>>>>>>> -fdump-xxx-yyy-start =A0 =A0 =A0 <-- dump before TODO_start >>>>>>>>> -fdump-xxx-yyy-before =A0 =A0<-- dump before main pass after TODO= _pass >>>>>>>>> -fdump-xxx-yyy-after =A0 =A0 =A0 <-- dump after main pass before = TODO_finish >>>>>>>>> -fdump-xxx-yyy-finish =A0 =A0 =A0<-- 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 >>>>>>> >>>>>>>>=A0"start" and "before" >>>>>>>> have no semantical difference to me ... as the dump before TODO_st= art >>>>>>>> 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 a= nd -finish >>>>>>>> (using your naming scheme). =A0Splitting 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 dest= roys >>>>>>>> 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). =A0I 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 confusi= on. >>>>>>>>>>>> >>>>>>>>>>>> + =A0/* Override dump TODOs. =A0*/ >>>>>>>>>>>> + =A0if (dump_file && (pass->todo_flags_finish & TODO_dump_fun= c) >>>>>>>>>>>> + =A0 =A0 =A0&& (dump_flags & TDF_BEFORE)) >>>>>>>>>>>> + =A0 =A0{ >>>>>>>>>>>> + =A0 =A0 =A0pass->todo_flags_finish &=3D ~TODO_dump_func; >>>>>>>>>>>> + =A0 =A0 =A0pass->todo_flags_start |=3D TODO_dump_func; >>>>>>>>>>>> + =A0 =A0} >>>>>>>>>>>> >>>>>>>>>>>> and certainly writing to pass is not ok. =A0And 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 ont= op >>>>>>>>>>>> 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 dumpi= ng if >>>>>>>>>>> user already asked for dumping -- it is annoying to see empty I= R 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 tim= e. Even >>>>>>>>>>> with all the dumps, you will need to eyeballing to find the pre= vious >>>>>>>>>>> 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. =A0Currently= some passes >>>>>>>>>> do not dump function bodies because they presumably do no IL >>>>>>>>>> modification. =A0But this is certainly the minority (and some pa= sses 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 afte= r todo-finish. >>>>>>>>>> By default we'd want after todo-finish. =A0When we no longer dum= p 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. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >