Jan Hubicka wrote:
This looks mostly fine to me. note that i added you to pr35094 since
this patch will resolve that issue.
I guess that one of the questions that i would have is why not have
there be a base structure for the core passmanager fields, and then a
union that contains a one of the tree, rtl or ipa specific fields.
My motivation on this that the passes for ipa/tree/rtl really do behave
differently. Especially the ipa passes. Right now you are doing this
with only macros and just throwing all of the info into one big
structure. This is not a problem from a space point of vied because
there are not enough passes to worry about, but it seems that the world
would actually be simpler if the structure knew if it was a ipa, tree or
rtl pass rather than just looking for the nulls.
Kenny
> Hi,
> based on the discussion, this is change I would like to do to the
> passmanager. I am sending the header change only first, because the
> actual change will need updating all PM datastructure initializers and
> compensate testsuite and documentation for the removal of RTL dump
> letters so I would rather do that just once. Does this seem OK?
>
> The patch include the read/write methods that will be just placeholders
> on mainline. Naturally I can remove them for time being at least as
> long as we think the RTL_PASS/TREE_PASS macros are good idea. I can
> quite easilly see that those are stepping back from plan not making
> passmanager poluted by ugly macros, but on the other hand since the PM
> is now doing RTL/IPA/tree passes it needs at least a little of
> flexibility to be able update API of one without affecting others.
>
> Alternative would be some simple inheriatance scheme: have the common
> fields in one structure and derrived tree/ipa/RTL pass structures.
> It complicates passes.c somewhat but I can definitly do that.
>
> Jan
>
> Index: tree-pass.h
> ===================================================================
> *** tree-pass.h (revision 133036)
> --- tree-pass.h (working copy)
> *************** extern const char *dump_file_name;
> *** 88,93 ****
> --- 88,97 ----
> /* Return the dump_file_info for the given phase. */
> extern struct dump_file_info *get_dump_file_info (enum tree_dump_index);
>
> + /* Forward declare so we don't need to bring in cgraph and varpool include.
> */
> + struct cgraph_node;
> + struct varpool_node;
> +
> /* Describe one pass. */
> struct tree_opt_pass
> {
> *************** struct tree_opt_pass
> *** 98,108 ****
> --- 102,129 ----
> the function returns true. */
> bool (*gate) (void);
>
> + /* IPA passes can analyze function body and variable initializers using
> this
> + hook and produce summary. */
> + void (*function_generate_summary) (struct cgraph_node *);
> + void (*variable_generate_summary) (struct varpool_node *);
> +
> + /* These hooks will be used to serialize IPA summaries on disk. For a
> moment
> + they are just placeholders. */
> + void (*function_write_summary) (struct cgraph_node *);
> + void (*variable_write_summary) (struct varpool_node *);
> + void (*function_read_summary) (struct cgraph_node *);
> + void (*variable_read_summary) (struct varpool_node *);
> +
> /* This is the code to run. If null, then there should be sub-passes
> otherwise this pass does nothing. The return value contains
> TODOs to execute in addition to those in TODO_flags_finish. */
> unsigned int (*execute) (void);
>
> + /* Results of interprocedural propagation of an IPA pass is applied to
> + function body via this hook. */
> + void (*function_transform) (struct cgraph_node *);
> + void (*variable_transform) (struct varpool_node *);
> +
> /* A list of sub-passes to run, dependent on gate predicate. */
> struct tree_opt_pass *sub;
>
> *************** struct tree_opt_pass
> *** 124,134 ****
> /* Flags indicating common sets things to do before and after. */
> unsigned int todo_flags_start;
> unsigned int todo_flags_finish;
> -
> - /* Letter for RTL dumps. */
> - char letter;
> };
>
> /* Define a tree dump switch. */
> struct dump_file_info
> {
> --- 145,166 ----
> /* Flags indicating common sets things to do before and after. */
> unsigned int todo_flags_start;
> unsigned int todo_flags_finish;
> };
>
> + /* RTL and tree passes are using just some of the hooks. The macros makes
> + it easier to add more hooks for different passes in future. */
> + #define RTL_PASS(execute) NULL, NULL, NULL, NULL, execute, NULL, NULL
> + #define TREE_PASS(execute) NULL, NULL, NULL, NULL, execute, NULL, NULL
> + /* IPA passes not using the generate_summary/execute/transform scheme and
> thus
> + not scalable for whole program optimization can use just execute hook.
> */
> + #define SIMPLE_IPA_PASS(execute) NULL, NULL, NULL, NULL, execute, NULL, NULL
> +
> + #define IPA_PASS_GENERATE_SUMMARY(fun,var) fun,var,
> + #define IPA_PASS_WRITE(fun,var) fun,var,
> + #define IPA_PASS_READ(fun,var) fun,var,
> + #define IPA_PASS_EXECUTE(execute) execute
> + #define IPA_PASS_TRANSFORM(fun,var) fun,var,
> +
> /* Define a tree dump switch. */
> struct dump_file_info
> {
> *************** struct dump_file_info
> *** 138,144 ****
> int flags; /* user flags */
> int state; /* state of play */
> int num; /* dump file number */
> - int letter; /* enabling letter for RTL dumps */
> };
>
> /* Pass properties. */
> --- 170,175 ----
>
