This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fwd: RFA: plugin events for ICI


Joern Rennecke wrote:
Quoting Basile STARYNKEVITCH <basile@starynkevitch.net>:
--- 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: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]