From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6853 invoked by alias); 30 Nov 2009 19:49:20 -0000 Received: (qmail 6845 invoked by uid 22791); 30 Nov 2009 19:49:19 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-115-monday.nerim.net (HELO maiev.nerim.net) (62.4.16.115) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 30 Nov 2009 19:49:15 +0000 Received: from hector.lesours (ours.starynkevitch.net [213.41.244.95]) by maiev.nerim.net (Postfix) with ESMTP id 5DFB9B82E8; Mon, 30 Nov 2009 20:49:12 +0100 (CET) Received: from glinka.lesours ([192.168.0.1]) by hector.lesours with esmtp (Exim 4.69) (envelope-from ) id 1NFCFI-0003If-V9; Mon, 30 Nov 2009 20:49:17 +0100 Message-ID: <4B1421D0.40004@starynkevitch.net> Date: Mon, 30 Nov 2009 20:12:00 -0000 From: Basile STARYNKEVITCH User-Agent: Mozilla-Thunderbird 2.0.0.22 (X11/20091109) MIME-Version: 1.0 To: Joern Rennecke CC: ctuning-discussions@googlegroups.com, gcc-patches , Grigori Fursin , 'Grigori Fursin' , 'Zbigniew Chamski' , 'Richard Guenther' , 'Ian Lance Taylor' , 'Albert Cohen' , 'Yuri Kashnikoff' , 'Yuanjie Huang' , 'Liang Peng' , dorit@il.ibm.com, 'Mircea Namolaru' , 'Diego Novillo' Subject: Re: Fwd: RFA: plugin events for ICI References: <20091130133452.hgxpnrkgu8cw0gk8-nzlynne@webmail.spamcop.net> <4B1418E2.9050400@starynkevitch.net> <20091130143921.vi6qzwej48so8o8c-nzlynne@webmail.spamcop.net> In-Reply-To: <20091130143921.vi6qzwej48so8o8c-nzlynne@webmail.spamcop.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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-11/txt/msg01739.txt.bz2 Joern Rennecke wrote: > Quoting Basile STARYNKEVITCH : >> --- gcc/gcc/doc/plugins.texi 2009-11-29 17:42:32.565901921 +0000 >> +++ gcc-ici-2/gcc/doc/plugins.texi 2009-11-30 16:35:59.804653957 +0000 >> @@ -156,18 +156,48 @@ enum plugin_event >> PLUGIN_ATTRIBUTES, /* Called during attribute >> registration */ >> PLUGIN_START_UNIT, /* Called before processing a >> translation unit. */ >> PLUGIN_PRAGMAS, /* Called during pragma registration. */ >> - PLUGIN_EVENT_LAST /* Dummy event used for indexing >> callback >> + /* Called to allow inspection and/or modification of unroll >> parameters. */ >> + PLUGIN_UNROLL_PARAMETER_HANDLER: >> >> Don't put colons here. You should have exactly the same code as in >> gcc/gcc-plugin.h > > Actually, there is only an include. The enumeration is in plugins.def, > and I had cut & pasted the list of new plugins from a switch statement. > But you are right, unless I want to change the entire list, I should put > a comma instead of a semicolon after each enumerator. > >> + PLUGIN_OVERRIDE_GATE: >> + /* Called before executing a pass. */ >> >> Why? > > The adapt plugin wants to enable passes independently of the original > gate function. E.g. with milepost this can be driven by machine learning. > >> + PLUGIN_PASS_EXECUTION: >> + /* Called before executing subpasses of a GIMPLE_PASS in >> + execute_ipa_pass_list. */ >> >> Why? > > This allows recording of the passes that are executed. > >> @@ -1491,9 +1500,21 @@ execute_one_pass (struct opt_pass *pass) >> >> current_pass = pass; >> >> - /* See if we're supposed to run this pass. */ >> - if (pass->gate && !pass->gate ()) >> + /* Check whether gate check should be avoided. >> + User controls the value of the gate through the parameter >> "gate_status". */ >> + gate_status = (pass->gate == NULL) ? true : pass->gate(); >> + >> + /* Override gate with plugin. */ >> + invoke_plugin_callbacks (PLUGIN_OVERRIDE_GATE, &gate_status); >> + >> >> This something I don't fully understand. Maybe it is just lacking >> documentation. 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. >> +/* Declare for plugins like ICI. */ >> +extern void do_per_function_toporder (void (*) (void *), void *); >> + >> >> The comment is not at all explanatory. Don't mention ICI or any >> specific plugin, but explain the reason behind the >> function. Please write several sentences, explaining what is the >> first function argument, and what is the second >> (perhaps some client data?). > > The function is explained in the passes.c, were it has been defined 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. 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). Thanks. Regards. -- Basile STARYNKEVITCH http://starynkevitch.net/Basile/ email: basilestarynkevitchnet mobile: +33 6 8501 2359 8, rue de la Faiencerie, 92340 Bourg La Reine, France *** opinions {are only mines, sont seulement les miennes} ***