Re: -fdump-passes -fenable-xxx=func_name_list

[Xinliang David Li <davidxl@google.com>]

[-- Attachment #1: Type: text/plain, Size: 5709 bytes --]

This is the patch with max id removed.

Thanks,

David

On Mon, Jun 6, 2011 at 9:00 AM, Xinliang David Li <davidxl@google.com> wrote:
> On Mon, Jun 6, 2011 at 4:38 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, Jun 2, 2011 at 9:12 AM, Xinliang David Li <davidxl@google.com> 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 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 <davidxl@google.com> wrote:
>>>> On Wed, Jun 1, 2011 at 12:29 PM, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Wed, Jun 1, 2011 at 6:16 PM, Xinliang David Li <davidxl@google.com> wrote:
>>>>>> On Wed, Jun 1, 2011 at 1:51 AM, Richard Guenther
>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>> On Wed, Jun 1, 2011 at 1:34 AM, Xinliang David Li <davidxl@google.com> 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

[-- Attachment #2: dump-pass4.p --]
[-- Type: text/x-pascal, Size: 7126 bytes --]

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 174550)
+++ 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
@@ -5060,7 +5061,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}
@@ -5483,6 +5485,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 174550)
+++ 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 174550)
+++ cgraphunit.c	(working copy)
@@ -1112,6 +1112,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 174550)
+++ 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 174550)
+++ 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,124 @@ 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, current_function_decl, 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)
+{
+  struct cgraph_node *n, *node = NULL;
+  tree save_fndecl = current_function_decl;
+
+  create_pass_tab();
+
+  n = cgraph_nodes;
+  while (n)
+    {
+      if (DECL_STRUCT_FUNCTION (n->decl))
+        {
+          node = n;
+          break;
+        }
+      n = n->next;
+    }
+
+  if (!node)
+    return;
+
+  push_cfun (DECL_STRUCT_FUNCTION (node->decl));
+  current_function_decl = node->decl;
+
+  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);
+
+  pop_cfun ();
+  current_function_decl = save_fndecl;
+}
+
+
 /* Returns the pass with NAME.  */
 
 static struct opt_pass *
@@ -513,9 +631,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)