The dump-pass patch with test case.
David
On Tue, Jun 7, 2011 at 11:54 AM, Xinliang David Li <[email protected]> wrote:
> Please review the attached two patches.
>
> In the first patch, gate functions are cleaned up. All the per
> function legality checks are moved into the executor and the
> optimization heuristic checks (optimize for size) remain in the
> gators. These allow the the following overriding order:
>
> common flags (O2, -ftree-vrp, -fgcse etc) <--- compiler
> heuristic (optimize for size/speed) <--- -fdisable/enable forcing pass
> options <--- legality check
>
> Testing under going. Ok for trunk?
>
> Thanks,
>
> David
>
> On Tue, Jun 7, 2011 at 9:24 AM, Xinliang David Li <[email protected]> wrote:
>> Ok -- that sounds good.
>>
>> David
>>
>> On Tue, Jun 7, 2011 at 3:10 AM, Richard Guenther
>> <[email protected]> wrote:
>>> On Mon, Jun 6, 2011 at 6:00 PM, Xinliang David Li <[email protected]>
>>> wrote:
>>>> On Mon, Jun 6, 2011 at 4:38 AM, Richard Guenther
>>>> <[email protected]> wrote:
>>>>> On Thu, Jun 2, 2011 at 9:12 AM, Xinliang David Li <[email protected]>
>>>>> wrote:
>>>>>> This is the version of the patch that walks through pass lists.
>>>>>>
>>>>>> Ok with this one?
>>>>>
>>>>> +/* Dump all optimization passes. */
>>>>> +
>>>>> +void
>>>>> +dump_passes (void)
>>>>> +{
>>>>> + struct cgraph_node *n, *node = NULL;
>>>>> + tree save_fndecl = current_function_decl;
>>>>> +
>>>>> + fprintf (stderr, "MAX_UID = %d\n", cgraph_max_uid);
>>>>>
>>>>> this isn't accurate info - cloning can cause more cgraph nodes to
>>>>> appear (it also looks completely unrelated to dump_passes ...).
>>>>> Please drop it.
>>>>
>>>> Ok.
>>>>
>>>>
>>>>>
>>>>> + create_pass_tab();
>>>>> + gcc_assert (pass_tab);
>>>>>
>>>>> you have quite many asserts of this kind - we don't want them when
>>>>> the previous stmt as in this case indicates everything is ok.
>>>>
>>>> ok.
>>>>
>>>>>
>>>>> + push_cfun (DECL_STRUCT_FUNCTION (node->decl));
>>>>>
>>>>> this has side-effects, I'm not sure we want this here. Why do you
>>>>> need it? Probably because of
>>>>>
>>>>> + is_really_on = override_gate_status (pass, current_function_decl,
>>>>> is_on);
>>>>>
>>>>> ? But that is dependent on the function given which should have no
>>>>> effect (unless it is overridden globally in which case
>>>>> override_gate_status
>>>>> and friends should deal with a NULL cfun).
>>>>
>>>> As we discussed, currently some pass gate functions depend on per node
>>>> information -- those checks need to be pushed into execute functions.
>>>> I would like to clean those up later -- at which time, the push/pop
>>>> can be removed.
>>>
>>> I'd like to do it the other way around, first clean up the gate functions
>>> then
>>> drop in this patch without the cfun push/pop. The revised patch looks ok
>>> to me with the cfun push/pop removed.
>>>
>>> Thanks,
>>> Richard.
>>>
>>>>>
>>>>> I don't understand why you need another table mapping pass to name
>>>>> when pass->name is available and the info is trivially re-constructible.
>>>>
>>>> This is needed as the pass->name is not the canonicalized name (i.e.,
>>>> not with number suffix etc), so the extra mapping from id to
>>>> normalized name is needed.
>>>>
>>>> Thanks,
>>>>
>>>> David
>>>>
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>> David
>>>>>>
>>>>>> On Wed, Jun 1, 2011 at 12:45 PM, Xinliang David Li <[email protected]>
>>>>>> wrote:
>>>>>>> On Wed, Jun 1, 2011 at 12:29 PM, Richard Guenther
>>>>>>> <[email protected]> wrote:
>>>>>>>> On Wed, Jun 1, 2011 at 6:16 PM, Xinliang David Li <[email protected]>
>>>>>>>> wrote:
>>>>>>>>> On Wed, Jun 1, 2011 at 1:51 AM, Richard Guenther
>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>> On Wed, Jun 1, 2011 at 1:34 AM, Xinliang David Li
>>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>>> The following patch implements the a new option that dumps gcc PASS
>>>>>>>>>>> configuration. The sample output is attached. There is one
>>>>>>>>>>> limitation: some placeholder passes that are named with '*xxx' are
>>>>>>>>>>> note registered thus they are not listed. They are not important as
>>>>>>>>>>> they can not be turned on/off anyway.
>>>>>>>>>>>
>>>>>>>>>>> The patch also enhanced -fenable-xxx and -fdisable-xx to allow a
>>>>>>>>>>> list
>>>>>>>>>>> of function assembler names to be specified.
>>>>>>>>>>>
>>>>>>>>>>> Ok for trunk?
>>>>>>>>>>
>>>>>>>>>> Please split the patch.
>>>>>>>>>>
>>>>>>>>>> I'm not too happy how you dump the pass configuration. Why not
>>>>>>>>>> simply,
>>>>>>>>>> at a _single_ place, walk the pass tree? Instead of doing pieces of
>>>>>>>>>> it
>>>>>>>>>> at pass execution time when it's not already dumped - that really
>>>>>>>>>> looks
>>>>>>>>>> gross.
>>>>>>>>>
>>>>>>>>> Yes, that was the original plan -- but it has problems
>>>>>>>>> 1) the dumper needs to know the root pass lists -- which can change
>>>>>>>>> frequently -- it can be a long term maintanance burden;
>>>>>>>>> 2) the centralized dumper needs to be done after option processing
>>>>>>>>> 3) not sure if gate functions have any side effects or have
>>>>>>>>> dependencies on cfun
>>>>>>>>>
>>>>>>>>> The proposed solutions IMHO is not that intrusive -- just three hooks
>>>>>>>>> to do the dumping and tracking indentation.
>>>>>>>>
>>>>>>>> Well, if you have a CU that is empty or optimized to nothing at some
>>>>>>>> point
>>>>>>>> you will not get a complete pass list. I suppose optimize attributes
>>>>>>>> might
>>>>>>>> also confuse output. Your solution might not be that intrusive
>>>>>>>> but it is still ugly. I don't see 1) as an issue, for 2) you can just
>>>>>>>> call the
>>>>>>>> dumping from toplev_main before calling do_compile (), 3) gate
>>>>>>>> functions
>>>>>>>> shouldn't have side-effects, but as they could gate on
>>>>>>>> optimize_for_speed ()
>>>>>>>> your option summary output will be bogus anyway.
>>>>>>>>
>>>>>>>> So - what is the output intended for if it isn't reliable?
>>>>>>>
>>>>>>> This needs to be cleaned up at some point -- the gate function should
>>>>>>> behave the same for all functions and per-function decisions need to
>>>>>>> be pushed down to the executor body. I will try to rework the patch
>>>>>>> as you suggested to see if there are problems.
>>>>>>>
>>>>>>> David
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The documentation should also link this option to the
>>>>>>>>>> -fenable/disable
>>>>>>>>>> options as obviously the pass names in that dump are those to be
>>>>>>>>>> used for those flags (and not readily available anywhere else).
>>>>>>>>>
>>>>>>>>> Ok.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I also think that it would be way more useful to note in the
>>>>>>>>>> individual
>>>>>>>>>> dump files the functions (at the place they would usually appear)
>>>>>>>>>> that
>>>>>>>>>> have the pass explicitly enabled/disabled.
>>>>>>>>>
>>>>>>>>> Ok -- for ipa passes or tree/rtl passes where all functions are
>>>>>>>>> explicitly disabled.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Richard.
>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi (revision 174759)
+++ doc/invoke.texi (working copy)
@@ -291,6 +291,7 @@ Objective-C and Objective-C++ Dialects}.
-fdump-translation-unit@r{[}-@var{n}@r{]} @gol
-fdump-class-hierarchy@r{[}-@var{n}@r{]} @gol
-fdump-ipa-all -fdump-ipa-cgraph -fdump-ipa-inline @gol
+-fdump-passes @gol
-fdump-statistics @gol
-fdump-tree-all @gol
-fdump-tree-original@r{[}-@var{n}@r{]} @gol
@@ -5069,7 +5070,8 @@ seperated list of function ranges. Each
The range is inclusive in both ends. If the range is trivial, the number pair can be
simplified a a single number. If the function's cgraph node's @var{uid} is falling
within one of the specified ranges, the @var{pass} is disabled for that function.
-The @var{uid} is shown in the function header of a dump file.
+The @var{uid} is shown in the function header of a dump file, and pass names can be
+dumped by using option @option{-fdump-passes}.
@item -fdisable-tree-@var{pass}
@item -fdisable-tree-@var{pass}=@var{range-list}
@@ -5492,6 +5494,11 @@ Dump after function inlining.
@end table
+@item -fdump-passes
+@opindex fdump-passes
+Dump the list of optimization passes that are turned on and off by
+the current command line options.
+
@item -fdump-statistics-@var{option}
@opindex fdump-statistics
Enable and control dumping of pass statistics in a separate file. The
Index: tree-pass.h
===================================================================
--- tree-pass.h (revision 174759)
+++ tree-pass.h (working copy)
@@ -639,5 +639,6 @@ extern void do_per_function_toporder (vo
extern void disable_pass (const char *);
extern void enable_pass (const char *);
+extern void dump_passes (void);
#endif /* GCC_TREE_PASS_H */
Index: cgraphunit.c
===================================================================
--- cgraphunit.c (revision 174759)
+++ cgraphunit.c (working copy)
@@ -1117,6 +1117,9 @@ cgraph_finalize_compilation_unit (void)
fflush (stderr);
}
+ if (flag_dump_passes)
+ dump_passes ();
+
/* Gimplify and lower all functions, compute reachability and
remove unreachable nodes. */
cgraph_analyze_functions ();
Index: common.opt
===================================================================
--- common.opt (revision 174759)
+++ common.opt (working copy)
@@ -1012,6 +1012,10 @@ fdump-noaddr
Common Report Var(flag_dump_noaddr)
Suppress output of addresses in debugging dumps
+fdump-passes
+Common Var(flag_dump_passes) Init(0)
+Dump optimization passes
+
fdump-unnumbered
Common Report Var(flag_dump_unnumbered)
Suppress output of instruction numbers, line number notes and addresses in debugging dumps
Index: passes.c
===================================================================
--- passes.c (revision 174759)
+++ passes.c (working copy)
@@ -478,7 +478,7 @@ passr_eq (const void *p1, const void *p2
return !strcmp (s1->unique_name, s2->unique_name);
}
-static htab_t pass_name_tab = NULL;
+static htab_t name_to_pass_map = NULL;
/* Register PASS with NAME. */
@@ -488,11 +488,11 @@ register_pass_name (struct opt_pass *pas
struct pass_registry **slot;
struct pass_registry pr;
- if (!pass_name_tab)
- pass_name_tab = htab_create (256, passr_hash, passr_eq, NULL);
+ if (!name_to_pass_map)
+ name_to_pass_map = htab_create (256, passr_hash, passr_eq, NULL);
pr.unique_name = name;
- slot = (struct pass_registry **) htab_find_slot (pass_name_tab, &pr, INSERT);
+ slot = (struct pass_registry **) htab_find_slot (name_to_pass_map, &pr, INSERT);
if (!*slot)
{
struct pass_registry *new_pr;
@@ -506,6 +506,101 @@ register_pass_name (struct opt_pass *pas
return; /* Ignore plugin passes. */
}
+/* Map from pass id to canonicalized pass name. */
+
+typedef const char *char_ptr;
+DEF_VEC_P(char_ptr);
+DEF_VEC_ALLOC_P(char_ptr, heap);
+static VEC(char_ptr, heap) *pass_tab = NULL;
+
+/* Callback function for traversing NAME_TO_PASS_MAP. */
+
+static int
+pass_traverse (void **slot, void *data ATTRIBUTE_UNUSED)
+{
+ struct pass_registry **p = (struct pass_registry **)slot;
+ struct opt_pass *pass = (*p)->pass;
+
+ gcc_assert (pass->static_pass_number > 0);
+ gcc_assert (pass_tab);
+
+ VEC_replace (char_ptr, pass_tab, pass->static_pass_number,
+ (*p)->unique_name);
+
+ return 1;
+}
+
+/* The function traverses NAME_TO_PASS_MAP and creates a pass info
+ table for dumping purpose. */
+
+static void
+create_pass_tab (void)
+{
+ if (!flag_dump_passes)
+ return;
+
+ VEC_safe_grow_cleared (char_ptr, heap,
+ pass_tab, passes_by_id_size + 1);
+ htab_traverse (name_to_pass_map, pass_traverse, NULL);
+}
+
+static bool override_gate_status (struct opt_pass *, tree, bool);
+
+/* Dump the instantiated name for PASS. IS_ON indicates if PASS
+ is turned on or not. */
+
+static void
+dump_one_pass (struct opt_pass *pass, int pass_indent)
+{
+ int indent = 3 * pass_indent;
+ const char *pn;
+ bool is_on, is_really_on;
+
+ is_on = (pass->gate == NULL) ? true : pass->gate();
+ is_really_on = override_gate_status (pass, NULL, is_on);
+
+ if (pass->static_pass_number <= 0)
+ pn = pass->name;
+ else
+ pn = VEC_index (char_ptr, pass_tab, pass->static_pass_number);
+
+ fprintf (stderr, "%*s%-40s%*s:%s%s\n", indent, " ", pn,
+ (15 - indent < 0 ? 0 : 15 - indent), " ",
+ is_on ? " ON" : " OFF",
+ ((!is_on) == (!is_really_on) ? ""
+ : (is_really_on ? " (FORCED_ON)" : " (FORCED_OFF)")));
+}
+
+/* Dump pass list PASS with indentation INDENT. */
+
+static void
+dump_pass_list (struct opt_pass *pass, int indent)
+{
+ do
+ {
+ dump_one_pass (pass, indent);
+ if (pass->sub)
+ dump_pass_list (pass->sub, indent + 1);
+ pass = pass->next;
+ }
+ while (pass);
+}
+
+/* Dump all optimization passes. */
+
+void
+dump_passes (void)
+{
+ create_pass_tab();
+
+ dump_pass_list (all_lowering_passes, 1);
+ dump_pass_list (all_small_ipa_passes, 1);
+ dump_pass_list (all_regular_ipa_passes, 1);
+ dump_pass_list (all_lto_gen_passes, 1);
+ dump_pass_list (all_passes, 1);
+}
+
+
/* Returns the pass with NAME. */
static struct opt_pass *
@@ -513,9 +608,8 @@ get_pass_by_name (const char *name)
{
struct pass_registry **slot, pr;
- gcc_assert (pass_name_tab);
pr.unique_name = name;
- slot = (struct pass_registry **) htab_find_slot (pass_name_tab,
+ slot = (struct pass_registry **) htab_find_slot (name_to_pass_map,
&pr, NO_INSERT);
if (!slot || !*slot)
Index: testsuite/gcc.dg/dump-pass.c
===================================================================
--- testsuite/gcc.dg/dump-pass.c (revision 0)
+++ testsuite/gcc.dg/dump-pass.c (revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-passes" } */
+
+unsigned res;
+
+void
+foo (unsigned code, int len)
+{
+ int i;
+ for (i = 0; i < len; i++)
+ res |= code & 1;
+}
+
+/* { dg-prune-output ".*" } */
