From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29126 invoked by alias); 1 Dec 2009 10:19:30 -0000 Received: (qmail 29115 invoked by uid 22791); 1 Dec 2009 10:19:29 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-pz0-f203.google.com (HELO mail-pz0-f203.google.com) (209.85.222.203) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 01 Dec 2009 10:19:25 +0000 Received: by pzk41 with SMTP id 41so3282803pzk.0 for ; Tue, 01 Dec 2009 02:19:24 -0800 (PST) MIME-Version: 1.0 Received: by 10.140.208.15 with SMTP id f15mr314506rvg.49.1259662764220; Tue, 01 Dec 2009 02:19:24 -0800 (PST) In-Reply-To: <20091201005430.4w42zvtxcwk4g0s4-nzlynne@webmail.spamcop.net> References: <20091130133452.hgxpnrkgu8cw0gk8-nzlynne@webmail.spamcop.net> <4B1418E2.9050400@starynkevitch.net> <20091130143921.vi6qzwej48so8o8c-nzlynne@webmail.spamcop.net> <4B1421D0.40004@starynkevitch.net> <20091201005430.4w42zvtxcwk4g0s4-nzlynne@webmail.spamcop.net> Date: Tue, 01 Dec 2009 10:23:00 -0000 Message-ID: <84fc9c000912010219n2dcf0103x635aeca1cbfbbf34@mail.gmail.com> Subject: Re: Fwd: RFA: plugin events for ICI From: Richard Guenther To: Joern Rennecke Cc: Basile STARYNKEVITCH , ctuning-discussions@googlegroups.com, gcc-patches , Grigori Fursin , Grigori Fursin , Zbigniew Chamski , Ian Lance Taylor , Albert Cohen , Yuri Kashnikoff , Yuanjie Huang , Liang Peng , dorit@il.ibm.com, Mircea Namolaru , 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: 2009-12/txt/msg00032.txt.bz2 On Tue, Dec 1, 2009 at 6:54 AM, Joern Rennecke wrote: > Quoting Basile STARYNKEVITCH : >> >> I am not sure to have understood all your explanation (but I admit >> being tired). >> Perhaps you should add an entire subsection in doc/plugins.texi >> explaining what you have added and how to use them. >> At the moment I am not sure to have understood how to use them. >> >> More generally, I believe plugins ought to be quite well documented. A >> good enough documentation is important to make them successful. > > I have extended plugins.texi to describe the new events. > >>> The function is explained in the passes.c, were it has been defined =A0= all >>> along. >> >> But then, please move the explanation to header files. I can assure you >> that newbies first read *.h files, and only when needed the *.c >> implementing them. > > The common practice in GCC is to document functions where they are > defined; most global functions are simply declared in a header file, > without any comment there. > I see no reason to deviate from this practice now. > >> I hope your patch will be accepted in 4.5. If I had the power to accept >> it, I'll ok it (provided you added more documentation). > > The patch with the updated documentation is attached. > I have tested the updated documentation with 'make info' and 'make pdf' . There is a lot of useful general cleanup mixed with more questionable changes. For one, comments like 'Changed for FICI0: ' or 'ICI Event: Substitution of pass manager.' do not belong in GCC source code. I do not particularly like the loop unroll changes (you have a pass start / end hook, why can't you modify the globals from there?). It smells like very specific to ICI. Likewise I do not like + /* ICI Event: Substitution of pass manager. */ + /* Try calling the event - if not successful, or if the plugin did not + manipulate passes, fall back on the default pass ordering. */ + substitute_status =3D 0; + invoke_plugin_callbacks (PLUGIN_ALL_PASSES_EXECUTION, &substitute_status= ); + if (substitute_status =3D=3D 0) + execute_pass_list (all_passes); + The rest looks reasonable, so if you can re-submit without the above changes...? Thanks, Richard.