From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17484 invoked by alias); 6 Jun 2011 15:59:44 -0000 Received: (qmail 17409 invoked by uid 22791); 6 Jun 2011 15:59:43 -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-wy0-f175.google.com (HELO mail-wy0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 06 Jun 2011 15:59:26 +0000 Received: by wye20 with SMTP id 20so3456191wye.20 for ; Mon, 06 Jun 2011 08:59:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.62.207 with SMTP id y15mr2775568wbh.86.1307375965416; Mon, 06 Jun 2011 08:59:25 -0700 (PDT) Received: by 10.227.37.152 with HTTP; Mon, 6 Jun 2011 08:59:25 -0700 (PDT) In-Reply-To: References: Date: Mon, 06 Jun 2011 15:59:00 -0000 Message-ID: Subject: Re: -fdump-passes -fenable-xxx=func_name_list From: Richard Guenther To: Xinliang David Li Cc: GCC Patches 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/msg00422.txt.bz2 On Mon, Jun 6, 2011 at 5:53 PM, Xinliang David Li wrot= e: > On Mon, Jun 6, 2011 at 4:22 AM, Richard Guenther > wrote: >> On Wed, Jun 1, 2011 at 7:24 PM, Xinliang David Li w= rote: >>> The attached is the split #1 patch that enhances -fenable/disable. >>> >>> Ok after testing? >> >> I expect the testcases will be quite fragile, so while I appreciate >> test coverage for new options I think we should go without those >> that involve any kind of UID. =A0Those which use assembler names >> also will fail randomly dependent on how targets mangle their >> functions - so I think we have to drop all testcases. > > Ok -- how about keeping tests with large uid range, and assembler name > for x86? A feature without testing is just to easy to break without > being noticed. That's true. Running the tests on a few selected known-good targets sounds good. Richard. >> >> Also >> >> +/* A helper function to determine if an identifier is valid to >> + =A0 be an assembler name (better to use target specific hook). =A0*/ >> + >> +static bool >> +is_valid_assembler_name (const char *str) >> +{ >> + =A0const char *p =3D str; >> + =A0char c; >> + >> + =A0c =3D *p; >> + =A0if (!((c >=3D 'a' && c <=3D 'z') >> + =A0 =A0 =A0 =A0|| (c >=3D 'A' && c <=3D 'Z') >> + =A0 =A0 =A0 =A0|| *p =3D=3D '_')) >> + =A0 =A0return false; >> + >> + =A0p++; >> + =A0while ((c =3D *p)) >> + =A0 { >> + =A0 =A0 if (!((c >=3D 'a' && c <=3D 'z') >> + =A0 =A0 =A0 =A0 =A0 || (c >=3D 'A' && c <=3D 'Z') >> + =A0 =A0 =A0 =A0 =A0 || (c >=3D '0' && c <=3D '9') >> + =A0 =A0 =A0 =A0 =A0 || *p =3D=3D '_')) >> + =A0 =A0 =A0 return false; >> + =A0 =A0 p++; >> + =A0 } >> + >> + =A0return true; >> +} >> >> why all that complicated checks? =A0Why not just check for p[0] >> in [^0-9] and re-structure the range parsing to switch between >> UIDs and assembler-names that way? > > Ok. > > David > >> >> Thanks, >> Richard. >> >>> Thanks, >>> David >>> >>> On Wed, Jun 1, 2011 at 9:16 AM, Xinliang David Li = wrote: >>>> On Wed, Jun 1, 2011 at 1:51 AM, Richard Guenther >>>> wrote: >>>>> On Wed, Jun 1, 2011 at 1:34 AM, Xinliang David Li wrote: >>>>>> The following patch implements the a new option that dumps gcc PASS >>>>>> configuration. The sample output is attached. =A0There is one >>>>>> limitation: some placeholder passes that are named with '*xxx' are >>>>>> note registered thus they are not listed. They are not important as >>>>>> they can not be turned on/off anyway. >>>>>> >>>>>> The patch also enhanced -fenable-xxx and -fdisable-xx to allow a list >>>>>> of function assembler names to be specified. >>>>>> >>>>>> Ok for trunk? >>>>> >>>>> Please split the patch. >>>>> >>>>> I'm not too happy how you dump the pass configuration. =A0Why not sim= ply, >>>>> at a _single_ place, walk the pass tree? =A0Instead of doing pieces o= f it >>>>> at pass execution time when it's not already dumped - that really loo= ks >>>>> gross. >>>> >>>> Yes, that was the original plan -- but it has problems >>>> 1) the dumper needs to know the root pass lists -- which can change >>>> frequently -- it can be a long term maintanance burden; >>>> 2) the centralized dumper needs to be done after option processing >>>> 3) not sure if gate functions have any side effects or have dependenci= es on cfun >>>> >>>> The proposed solutions IMHO is not that intrusive -- just three hooks >>>> to do the dumping and tracking indentation. >>>> >>>>> >>>>> The documentation should also link this option to the -fenable/disable >>>>> options as obviously the pass names in that dump are those to be >>>>> used for those flags (and not readily available anywhere else). >>>> >>>> Ok. >>>> >>>>> >>>>> I also think that it would be way more useful to note in the individu= al >>>>> dump files the functions (at the place they would usually appear) that >>>>> have the pass explicitly enabled/disabled. >>>> >>>> Ok -- for ipa passes or tree/rtl passes where all functions are >>>> explicitly disabled. >>>> >>>> Thanks, >>>> >>>> David >>>> >>>>> >>>>> Richard. >>>>> >>>>>> Thanks, >>>>>> >>>>>> David >>>>>> >>>>> >>>> >>> >> >