From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2163 invoked by alias); 8 Jun 2011 16:41:39 -0000 Received: (qmail 2153 invoked by uid 22791); 8 Jun 2011 16:41:38 -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; Wed, 08 Jun 2011 16:41:22 +0000 Received: from hpaq5.eem.corp.google.com (hpaq5.eem.corp.google.com [172.25.149.5]) by smtp-out.google.com with ESMTP id p58GfKd4007075 for ; Wed, 8 Jun 2011 09:41:21 -0700 Received: from yie12 (yie12.prod.google.com [10.243.66.12]) by hpaq5.eem.corp.google.com with ESMTP id p58Gf9CQ018864 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NOT) for ; Wed, 8 Jun 2011 09:41:19 -0700 Received: by yie12 with SMTP id 12so491818yie.11 for ; Wed, 08 Jun 2011 09:41:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.150.170.2 with SMTP id s2mr984101ybe.16.1307551278763; Wed, 08 Jun 2011 09:41:18 -0700 (PDT) Received: by 10.151.26.21 with HTTP; Wed, 8 Jun 2011 09:41:18 -0700 (PDT) In-Reply-To: References: <20110601231202.224188ad.basile@starynkevitch.net> Date: Wed, 08 Jun 2011 16:52: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/msg00660.txt.bz2 On Wed, Jun 8, 2011 at 2:06 AM, Richard Guenther wrote: > On Wed, Jun 8, 2011 at 1:08 AM, Xinliang David Li wr= ote: >> 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_fi= nish >> -fdump-xxx-yyy-finish =A0 =A0 =A0<-- dump after TODO_finish > > Can we bikeshed a bit more about these names? Yes. >"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? But the key usefulness here is pass isolation -- you don't need to know the phase ordering -- the 'before' and 'after' are relative to the pass that is specified. > If you'd specify it for a single pass then you'd get both -start and -fin= ish > (using your naming scheme). =A0Splitting that dump(s) to different files > then might make sense (not sure about the name to use). > The only downside is it loses fine grain control --- e.g, split IR emission for the any one of the dumping points (before start todo, before pass, after pass, after finish todo) into a separate file when that point is specified. The content of the default dumping remains unchanged unless the 'finish' point is specified explicitly. In short, the patch I had has minimal impact on default behavior, just adding the capability to extract clean IR into different files for diffing purpose. > 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). =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 don't quite understand this comment. The processing order of functions are kept in the split files.The main purpose of splitting is to ease diffing for any transformation -- and our current dump can be really hard to parse :) > > 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. Yes -- that can be done. Thanks, David > > 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 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. =A0Currently some p= asses >>> do not dump function bodies because they presumably do no IL >>> modification. =A0But 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. =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. >>>>> >>>> >>> >> >