This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Name unnamed passes and change duplicate pass names
Thanks for pointing me in the right direction. It turns out that
naming unnamed passes does indeed create unnecessary dump files.
These pass files do not include dumps, however, only ;; Function
headers. Sorry for the late response, by the way; it took me longer
than I expected to learn about the dump file system.
I think the best solution is to decouple the decision about whether a
pass should have a dump file from whether or not it is named (so we
can then name every pass).
I added the function needs_dump_file() to passes.c for this purpose:
it indicates that a pass needs a dump file if it requests a dump with
TODO_dump_func or TODO_dump_cgraph. I also added a
TODO_require_dumpfile, which lets a pass indicate that it needs a dump
file without TODO_dump_func or TODO_dump_cgraph.
I'll provide an additional patch that adds TODO_require_dumpfile to
passes that need it. I first thought that creating dump files for
passes with TODO_dump_func and TODO_dump_cgraph would be sufficient,
but it turns out that some passes without either of those flags output
other information to their dump files.
I did a before-and-after spot check to make sure dump output stays the
same. There are a few places where I allowed the patched version's
behavior to deviate because it's probably more correct.
These passes already had names before I started but do not generate
any dump output that I could find:
I assume that these dump files are superfluous, so I don't plan to
give them TODO_require_dumpfile flags.
Also, pass_referenced_vars previously did not generate a dump file (as
it was unnamed) but now does because it is marked TODO_dump_func. I
assume that generating the dump is actually the desired behavior, so I
left that as it.
Finally, I got rid of all places that check if pass->name is NULL. I
replaced these checks with a gcc_assert(). I can get rid of that if
it's too radical a change.
2009-05-12 Justin Seyster <email@example.com>
* passes.c (register_dump_files_1, next_pass_1): Don't
use (pass->name != NULL) to decide if a pass needs a dump file.
(execute_one_pass): Remove special case for NULL pass->name.
* tree-pass.h: Added TODO_require_dumpfile flag.
* statistics.c (curr_statistics_hash): Added assert to make sure
that a negative static_pasS_number never gets implicitly cast to
an unsigned int.
--- gcc/passes.c (revision 147136)
+++ gcc/passes.c (working copy)
@@ -396,6 +396,26 @@
set_pass_for_id (id, pass);
+/* True if a set of TODO flags include any option that will output to
+ a dump file. */
+todo_includes_dump (unsigned int flags)
+ return ((flags & TODO_dump_cgraph) || (flags & TODO_dump_func)
+ || (flags & TODO_require_dumpfile));
+/* True if a pass needs a dump file because it wants to dump call
+ graphs or functions. */
+needs_dump_file (struct opt_pass *pass)
+ return (todo_includes_dump (pass->todo_flags_start)
+ || todo_includes_dump (pass->todo_flags_finish));
/* Recursive worker function for register_dump_files. */
@@ -406,7 +426,8 @@
int new_properties = (properties | pass->properties_provided)
- if (pass->name && pass->name != '*')
+ gcc_assert (pass->name);
+ if (needs_dump_file (pass) && pass->name != '*')
@@ -459,7 +480,7 @@
and so it should rename the dump file. The first instance will
be -1, and be number of duplicates = -static_pass_number - 1.
Subsequent instances will be > 0 and just the duplicate number. */
- if (pass->name)
+ if (needs_dump_file (pass))
pass->static_pass_number -= 1;
new_pass->static_pass_number = -pass->static_pass_number;
@@ -1261,8 +1282,9 @@
if (pass->gate && !pass->gate ())
+ gcc_assert (pass->name);
if (!quiet_flag && !cfun)
- fprintf (stderr, " <%s>", pass->name ? pass->name : "");
+ fprintf (stderr, " <%s>", pass->name);
if (pass->todo_flags_start & TODO_set_props)
cfun->curr_properties = pass->properties_required;
--- gcc/tree-pass.h (revision 147136)
+++ gcc/tree-pass.h (working copy)
@@ -293,6 +293,10 @@
/* Rebuild the addressable-vars bitmap and do register promotion. */
#define TODO_update_address_taken (1 << 21)
+/* Dummy TODO that forces register_dump_files to create a dump file
+ even without TODO_dump_func or TODO_dump_cgraph. */
+#define TODO_require_dumpfile (1 << 22)
#define TODO_update_ssa_any \
| TODO_update_ssa_no_phi \
--- gcc/statistics.c (revision 147136)
+++ gcc/statistics.c (working copy)
@@ -82,8 +82,11 @@
- unsigned idx = current_pass->static_pass_number;
+ unsigned idx;
+ gcc_assert(current_pass->static_pass_number > 0);
+ idx = current_pass->static_pass_number;
if (idx < nr_statistics_hashes
&& statistics_hashes[idx] != NULL)
On Fri, May 8, 2009 at 4:29 AM, Paolo Bonzini <firstname.lastname@example.org> wrote:
>> Does this cause dumps to happen even if there is nothing to dump?
> Yes, it does.
>> Maybe you need to add another field to the struct saying a dump should
>> be created.
> Or another TODO (since it should print the ";; Function" header in
> dump_file, having something like TODO_dump_header in todo_start makes
> sort of sense; and set dump_file == NULL if the TODO is not present so
> that having TODO_dump_func without TODO_dump_header will crash).