> 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.
Unions are impossible to initialize by static initializers as we use
them now. One can have the common part as contained structure and do
some "poor mans inheritance" in C instead. That is have "pass"
structure containing the basic stuff and then "tree_pass", "rtl_pass"
and "ipa_pass" with the specific bits added contianing "pass" structure
as first field. The initializers then would have extra brackets around
and the generic bits of passmanager can use pointers to "pass" that
would be casted to "tree_pass"/"rtl_pass" and such. It would also imply
that the "execute" hook of IPA pass would appear earlier than the
"generate_summary"/"read"/"write" that are logically executed before it,
but it is not big deal.
I can do that way too. For start the tree_pass/rtl_pass probably would
contain nothing, as I think I've killed last different field (the
letter bit)
>
> 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.
Some way to differntitate the pass types would be good. At the moment
we can do that based on properties structure and the context, but it is
a bit weak.
I can definitly add an enum specifying the type (RTL/TREE/SMALL_IPA/IPA_PASS)
Honza
>
> 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 ----
> >
