From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9531 invoked by alias); 10 Jun 2011 16:22:39 -0000 Received: (qmail 9519 invoked by uid 22791); 10 Jun 2011 16:22:37 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 10 Jun 2011 16:22:21 +0000 Received: from kpbe15.cbf.corp.google.com (kpbe15.cbf.corp.google.com [172.25.105.79]) by smtp-out.google.com with ESMTP id p5AGMKRt011550 for ; Fri, 10 Jun 2011 09:22:20 -0700 Received: from vxb37 (vxb37.prod.google.com [10.241.33.101]) by kpbe15.cbf.corp.google.com with ESMTP id p5AGMJWi031884 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Fri, 10 Jun 2011 09:22:19 -0700 Received: by vxb37 with SMTP id 37so2644606vxb.4 for ; Fri, 10 Jun 2011 09:22:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.151.42.9 with SMTP id u9mr3336543ybj.244.1307722938730; Fri, 10 Jun 2011 09:22:18 -0700 (PDT) Received: by 10.151.26.21 with HTTP; Fri, 10 Jun 2011 09:22:18 -0700 (PDT) In-Reply-To: References: <20110601231202.224188ad.basile@starynkevitch.net> Date: Fri, 10 Jun 2011 16:54:00 -0000 Message-ID: Subject: Re: Dump before flag From: Xinliang David Li To: Richard Guenther Cc: GCC Patches , Diego Novillo Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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/msg00859.txt.bz2 On Fri, Jun 10, 2011 at 1:52 AM, Richard Guenther wrote: > On Thu, Jun 9, 2011 at 5:47 PM, Xinliang David Li wr= ote: >> 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 a= re >>>>>>> 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_p= ass >>>>>>> -fdump-xxx-yyy-after =A0 =A0 =A0 <-- dump after main pass before TO= DO_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_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). =A0Splitting that dump(s) to different f= iles >>>>>> 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 destro= ys >>>>>> 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 confusion. >>>>>>>>>> >>>>>>>>>> + =A0/* Override dump TODOs. =A0*/ >>>>>>>>>> + =A0if (dump_file && (pass->todo_flags_finish & TODO_dump_func) >>>>>>>>>> + =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 f= lag >>>>>>>>>> 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 ea= sily >>>>>>>>> -- 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 p= ass, >>>>>>>>> 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 previ= ous >>>>>>>>> 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 s= ome passes >>>>>>>> do not dump function bodies because they presumably do no IL >>>>>>>> modification. =A0But this is certainly the minority (and some pass= es 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. =A0When 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. >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >