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

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

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

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 <davidxl@google.com> wrote:
> Ok -- that sounds good.
>
> David
>
> On Tue, Jun 7, 2011 at 3:10 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Mon, Jun 6, 2011 at 6:00 PM, 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'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 <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: gate_cleanup.p --]
[-- Type: text/x-pascal, Size: 6445 bytes --]

Index: tree-complex.c
===================================================================
--- tree-complex.c	(revision 174759)
+++ tree-complex.c	(working copy)
@@ -1569,6 +1569,11 @@ tree_lower_complex (void)
   gimple_stmt_iterator gsi;
   basic_block bb;
 
+  /* With errors, normal optimization passes are not run.  If we don't
+     lower complex operations at all, rtl expansion will abort.  */
+  if (cfun->curr_properties & PROP_gimple_lcx)
+    return 0;
+
   if (!init_dont_simulate_again ())
     return 0;
 
@@ -1634,9 +1639,7 @@ struct gimple_opt_pass pass_lower_comple
 static bool
 gate_no_optimization (void)
 {
-  /* With errors, normal optimization passes are not run.  If we don't
-     lower complex operations at all, rtl expansion will abort.  */
-  return !(cfun->curr_properties & PROP_gimple_lcx);
+  return true;
 }
 
 struct gimple_opt_pass pass_lower_complex_O0 =
Index: tree-stdarg.c
===================================================================
--- tree-stdarg.c	(revision 174759)
+++ tree-stdarg.c	(working copy)
@@ -627,8 +627,7 @@ check_all_va_list_escapes (struct stdarg
 static bool
 gate_optimize_stdarg (void)
 {
-  /* This optimization is only for stdarg functions.  */
-  return cfun->stdarg != 0;
+  return true;
 }
 
 
@@ -645,6 +644,10 @@ execute_optimize_stdarg (void)
   const char *funcname = NULL;
   tree cfun_va_list;
 
+  /* This optimization is only for stdarg functions.  */
+  if (cfun->stdarg == 0)
+    return 0;
+
   cfun->va_list_gpr_size = 0;
   cfun->va_list_fpr_size = 0;
   memset (&si, 0, sizeof (si));
Index: tree-eh.c
===================================================================
--- tree-eh.c	(revision 174759)
+++ tree-eh.c	(working copy)
@@ -3234,6 +3234,9 @@ execute_lower_eh_dispatch (void)
   bool any_rewritten = false;
   bool redirected = false;
 
+  if (cfun->eh->region_tree == NULL)
+    return 0;
+
   assign_filter_values ();
 
   FOR_EACH_BB (bb)
@@ -3254,7 +3257,7 @@ execute_lower_eh_dispatch (void)
 static bool
 gate_lower_eh_dispatch (void)
 {
-  return cfun->eh->region_tree != NULL;
+  return true;
 }
 
 struct gimple_opt_pass pass_lower_eh_dispatch =
@@ -3983,8 +3986,12 @@ execute_cleanup_eh_1 (void)
 static unsigned int
 execute_cleanup_eh (void)
 {
-  int ret = execute_cleanup_eh_1 ();
+  int ret;
 
+  if (cfun->eh == NULL || cfun->eh->region_tree == NULL)
+    return 0;
+
+  ret = execute_cleanup_eh_1 ();
   /* If the function no longer needs an EH personality routine
      clear it.  This exposes cross-language inlining opportunities
      and avoids references to a never defined personality routine.  */
@@ -3998,7 +4005,7 @@ execute_cleanup_eh (void)
 static bool
 gate_cleanup_eh (void)
 {
-  return cfun->eh != NULL && cfun->eh->region_tree != NULL;
+  return true;
 }
 
 struct gimple_opt_pass pass_cleanup_eh = {
Index: gcse.c
===================================================================
--- gcse.c	(revision 174759)
+++ gcse.c	(working copy)
@@ -3713,15 +3713,17 @@ static bool
 gate_rtl_pre (void)
 {
   return optimize > 0 && flag_gcse
-    && !cfun->calls_setjmp
-    && optimize_function_for_speed_p (cfun)
-    && dbg_cnt (pre);
+         && optimize_function_for_speed_p (cfun);
 }
 
 static unsigned int
 execute_rtl_pre (void)
 {
   int changed;
+
+  if (cfun->calls_setjmp || !dbg_cnt (pre))
+    return 0;
+
   delete_unreachable_blocks ();
   df_analyze ();
   changed = one_pre_gcse_pass ();
@@ -3735,18 +3737,20 @@ static bool
 gate_rtl_hoist (void)
 {
   return optimize > 0 && flag_gcse
-    && !cfun->calls_setjmp
-    /* It does not make sense to run code hoisting unless we are optimizing
-       for code size -- it rarely makes programs faster, and can make then
-       bigger if we did PRE (when optimizing for space, we don't run PRE).  */
-    && optimize_function_for_size_p (cfun)
-    && dbg_cnt (hoist);
+        /* It does not make sense to run code hoisting unless we are optimizing
+         for code size -- it rarely makes programs faster, and can make then
+         bigger if we did PRE (when optimizing for space, we don't run PRE).  */
+        && optimize_function_for_size_p (cfun);
 }
 
 static unsigned int
 execute_rtl_hoist (void)
 {
   int changed;
+
+  if (cfun->calls_setjmp || !dbg_cnt (hoist))
+      return 0;
+
   delete_unreachable_blocks ();
   df_analyze ();
   changed = one_code_hoisting_pass ();
@@ -3799,4 +3803,3 @@ struct rtl_opt_pass pass_rtl_hoist =
 };
 
 #include "gt-gcse.h"
-
Index: except.c
===================================================================
--- except.c	(revision 174759)
+++ except.c	(working copy)
@@ -1440,14 +1440,17 @@ finish_eh_generation (void)
 static bool
 gate_handle_eh (void)
 {
-  /* Nothing to do if no regions created.  */
-  return cfun->eh->region_tree != NULL;
+  return true;
 }
 
 /* Complete generation of exception handling code.  */
 static unsigned int
 rest_of_handle_eh (void)
 {
+  /* Nothing to do if no regions created.  */
+  if (cfun->eh->region_tree == NULL)
+    return 0;
+
   finish_eh_generation ();
   cleanup_cfg (CLEANUP_NO_INSN_DEL);
   return 0;
@@ -2392,6 +2395,9 @@ convert_to_eh_region_ranges (void)
   int min_labelno = 0, max_labelno = 0;
   int saved_call_site_base = call_site_base;
 
+  if (cfun->eh->region_tree == NULL)
+    return 0;
+
   crtl->eh.action_record_data = VEC_alloc (uchar, gc, 64);
 
   ar_hash = htab_create (31, action_record_hash, action_record_eq, free);
@@ -2643,8 +2649,6 @@ static bool
 gate_convert_to_eh_region_ranges (void)
 {
   /* Nothing to do for SJLJ exceptions or if no regions created.  */
-  if (cfun->eh->region_tree == NULL)
-    return false;
   if (targetm.except_unwind_info (&global_options) == UI_SJLJ)
     return false;
   return true;
Index: cprop.c
===================================================================
--- cprop.c	(revision 174759)
+++ cprop.c	(working copy)
@@ -1843,15 +1843,17 @@ one_cprop_pass (void)
 static bool
 gate_rtl_cprop (void)
 {
-  return optimize > 0 && flag_gcse
-    && !cfun->calls_setjmp
-    && dbg_cnt (cprop);
+  return optimize > 0 && flag_gcse;
 }
 
 static unsigned int
 execute_rtl_cprop (void)
 {
   int changed;
+
+  if (cfun->calls_setjmp || !dbg_cnt (cprop))
+    return 0;
+
   delete_unreachable_blocks ();
   df_set_flags (DF_LR_RUN_DCE);
   df_analyze ();
@@ -1882,4 +1884,3 @@ struct rtl_opt_pass pass_rtl_cprop =
   TODO_verify_flow | TODO_ggc_collect   /* todo_flags_finish */
  }
 };
-

[-- Attachment #3: dump-pass5.p --]
[-- Type: text/x-pascal, Size: 6670 bytes --]

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)