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

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

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

Please take a look at the revised one.

Thanks,

David

On Mon, Jun 6, 2011 at 4:22 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jun 1, 2011 at 7:24 PM, Xinliang David Li <davidxl@google.com> wrote:
>> The attached is the split #1 patch that enhances -fenable/disable.
>>
>> Ok after testing?
>
> I expect the testcases will be quite fragile, so while I appreciate
> test coverage for new options I think we should go without those
> that involve any kind of UID.  Those which use assembler names
> also will fail randomly dependent on how targets mangle their
> functions - so I think we have to drop all testcases.
>
> Also
>
> +/* A helper function to determine if an identifier is valid to
> +   be an assembler name (better to use target specific hook).  */
> +
> +static bool
> +is_valid_assembler_name (const char *str)
> +{
> +  const char *p = str;
> +  char c;
> +
> +  c = *p;
> +  if (!((c >= 'a' && c <= 'z')
> +        || (c >= 'A' && c <= 'Z')
> +        || *p == '_'))
> +    return false;
> +
> +  p++;
> +  while ((c = *p))
> +   {
> +     if (!((c >= 'a' && c <= 'z')
> +           || (c >= 'A' && c <= 'Z')
> +           || (c >= '0' && c <= '9')
> +           || *p == '_'))
> +       return false;
> +     p++;
> +   }
> +
> +  return true;
> +}
>
> why all that complicated checks?  Why not just check for p[0]
> in [^0-9] and re-structure the range parsing to switch between
> UIDs and assembler-names that way?
>
> Thanks,
> Richard.
>
>> Thanks,
>> David
>>
>> On Wed, Jun 1, 2011 at 9:16 AM, 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.
>>>
>>>>
>>>> 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: enable-disable-funcname2.p --]
[-- Type: text/x-pascal, Size: 12575 bytes --]

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 174549)
+++ doc/invoke.texi	(working copy)
@@ -5056,11 +5056,12 @@ appended with a sequential number starti
 Disable rtl pass @var{pass}.  @var{pass} is the pass name.  If the same pass is
 statically invoked in the compiler multiple times, the pass name should be
 appended with a sequential number starting from 1.  @var{range-list} is a comma
-seperated list of function ranges.  Each range is a number pair seperated by a colon.
-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.
+seperated list of function ranges or assembler names.  Each range is a number
+pair seperated by a colon.  The range is inclusive in both ends.  If the range
+is trivial, the number pair can be simplified as 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.
 
 @item -fdisable-tree-@var{pass}
 @item -fdisable-tree-@var{pass}=@var{range-list}
@@ -5090,7 +5091,8 @@ of option arguments.
    -fenable-tree-cunroll=1
 # disable gcse2 for functions at the following ranges [1,1],
 # [300,400], and [400,1000]
-   -fdisable-rtl-gcse2=1:100,300,400:1000
+# disable gcse2 for functions foo and foo2
+   -fdisable-rtl-gcse2=foo,foo2
 # disable early inlining
    -fdisable-tree-einline
 # disable ipa inlining
Index: testsuite/gcc.dg/inline_2.c
===================================================================
--- testsuite/gcc.dg/inline_2.c	(revision 0)
+++ testsuite/gcc.dg/inline_2.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=0:100 -fdisable-ipa-inline" } */
+int g;
+__attribute__((always_inline)) void bar (void)
+{
+  g++;
+}
+
+int foo (void)
+{
+  bar ();
+  return g;
+}
+
+int foo2 (void)
+{
+  bar();
+  return g + 1;
+}
+
+/* { dg-final { scan-tree-dump-times "bar" 5 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/unroll_2.c
===================================================================
--- testsuite/gcc.dg/unroll_2.c	(revision 0)
+++ testsuite/gcc.dg/unroll_2.c	(revision 0)
@@ -0,0 +1,33 @@
+/* { dg-do compile  { target i?86-*-linux* x86_64-*-linux* } } */
+/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll=foo -fdisable-tree-cunrolli=foo -fenable-rtl-loop2_unroll" } */
+
+unsigned a[100], b[100];
+inline void bar()
+{
+ a[10] = b[10];
+}
+
+int foo(void)
+{
+  int i;
+  bar();
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+int foo2(void)
+{
+  int i;
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+/* { dg-final { scan-rtl-dump-times "Decided to peel loop completely" 1 "loop2_unroll" } } */
+/* { dg-final { cleanup-rtl-dump "loop2_unroll" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/inline_3.c
===================================================================
--- testsuite/gcc.dg/inline_3.c	(revision 0)
+++ testsuite/gcc.dg/inline_3.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile   { target i?86-*-linux* x86_64-*-linux* } } */
+/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=foo,foo2 -fdisable-ipa-inline" } */
+int g;
+__attribute__((always_inline)) void bar (void)
+{
+  g++;
+}
+
+int foo (void)
+{
+  bar ();
+  return g;
+}
+
+int foo2 (void)
+{
+  bar();
+  return g + 1;
+}
+
+/* { dg-final { scan-tree-dump-times "bar" 5 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/unroll_3.c
===================================================================
--- testsuite/gcc.dg/unroll_3.c	(revision 0)
+++ testsuite/gcc.dg/unroll_3.c	(revision 0)
@@ -0,0 +1,33 @@
+/* { dg-do compile  { target i?86-*-linux* x86_64-*-linux* } } */
+/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll=foo" } */
+
+unsigned a[100], b[100];
+inline void bar()
+{
+ a[10] = b[10];
+}
+
+int foo(void)
+{
+  int i;
+  bar();
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+int foo2(void)
+{
+  int i;
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+/* { dg-final { scan-rtl-dump-times "Decided to peel loop completely" 1 "loop2_unroll" } } */
+/* { dg-final { cleanup-rtl-dump "loop2_unroll" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/inline_4.c
===================================================================
--- testsuite/gcc.dg/inline_4.c	(revision 0)
+++ testsuite/gcc.dg/inline_4.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile  { target i?86-*-linux* x86_64-*-linux* } } */
+/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=foo2 -fdisable-ipa-inline" } */
+int g;
+__attribute__((always_inline)) void bar (void)
+{
+  g++;
+}
+
+int foo (void)
+{
+  bar ();
+  return g;
+}
+
+int foo2 (void)
+{
+  bar();
+  return g + 1;
+}
+
+/* { dg-final { scan-tree-dump-times "bar" 4 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/unroll_4.c
===================================================================
--- testsuite/gcc.dg/unroll_4.c	(revision 0)
+++ testsuite/gcc.dg/unroll_4.c	(revision 0)
@@ -0,0 +1,33 @@
+/* { dg-do compile  { target i?86-*-linux* x86_64-*-linux* } } */
+/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll=foo2" } */
+
+unsigned a[100], b[100];
+inline void bar()
+{
+ a[10] = b[10];
+}
+
+int foo(void)
+{
+  int i;
+  bar();
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+int foo2(void)
+{
+  int i;
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+/* { dg-final { scan-rtl-dump-times "Decided to peel loop completely" 1 "loop2_unroll" } } */
+/* { dg-final { cleanup-rtl-dump "loop2_unroll" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/inline_1.c
===================================================================
--- testsuite/gcc.dg/inline_1.c	(revision 0)
+++ testsuite/gcc.dg/inline_1.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline -fdisable-ipa-inline" } */
+int g;
+__attribute__((always_inline)) void bar (void)
+{
+  g++;
+}
+
+int foo (void)
+{
+  bar ();
+  return g;
+}
+
+int foo2 (void)
+{
+  bar();
+  return g + 1;
+}
+
+/* { dg-final { scan-tree-dump-times "bar" 5 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+/* { dg-excess-errors "extra notes" } */
Index: testsuite/gcc.dg/unroll_1.c
===================================================================
--- testsuite/gcc.dg/unroll_1.c	(revision 0)
+++ testsuite/gcc.dg/unroll_1.c	(revision 0)
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll" } */
+
+unsigned a[100], b[100];
+inline void bar()
+{
+ a[10] = b[10];
+}
+
+int foo(void)
+{
+  int i;
+  bar();
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+int foo2(void)
+{
+  int i;
+  for (i = 0; i < 2; i++)
+  {
+     a[i]= b[i] + 1;
+  }
+  return 1;
+}
+
+/* { dg-final { scan-rtl-dump-times "Decided to peel loop completely" 2 "loop2_unroll" } } */
+/* { dg-final { cleanup-rtl-dump "loop2_unroll" } } */
+/* { dg-excess-errors "extra notes" } */
Index: passes.c
===================================================================
--- passes.c	(revision 174549)
+++ passes.c	(working copy)
@@ -531,6 +531,7 @@ struct uid_range
 {
   unsigned int start;
   unsigned int last;
+  const char *assem_name;
   struct uid_range *next;
 };
 
@@ -542,6 +543,7 @@ DEF_VEC_ALLOC_P(uid_range_p, heap);
 static VEC(uid_range_p, heap) *enabled_pass_uid_range_tab = NULL;
 static VEC(uid_range_p, heap) *disabled_pass_uid_range_tab = NULL;
 
+
 /* Parse option string for -fdisable- and -fenable-
    The syntax of the options:
 
@@ -628,6 +630,7 @@ enable_disable_pass (const char *arg, bo
 	  uid_range_p new_range;
 	  char *invalid = NULL;
 	  long start;
+	  char *func_name = NULL;
 
 	  next_range = strchr (one_range, ',');
 	  if (next_range)
@@ -645,17 +648,31 @@ enable_disable_pass (const char *arg, bo
 	  start = strtol (one_range, &invalid, 10);
 	  if (*invalid || start < 0)
 	    {
-	      error ("Invalid range %s in option %s",
-		     one_range,
-		     is_enable ? "-fenable" : "-fdisable");
-	      free (argstr);
-	      return;
+              if (end_val || (one_range[0] >= '0'
+			      && one_range[0] <= '9'))
+                {
+                  error ("Invalid range %s in option %s",
+                         one_range,
+                         is_enable ? "-fenable" : "-fdisable");
+                  free (argstr);
+                  return;
+                }
+	      func_name = one_range;
 	    }
 	  if (!end_val)
 	    {
 	      new_range = XCNEW (struct uid_range);
-	      new_range->start = (unsigned) start;
-	      new_range->last = (unsigned) start;
+              if (!func_name)
+                {
+                  new_range->start = (unsigned) start;
+                  new_range->last = (unsigned) start;
+                }
+              else
+                {
+                  new_range->start = (unsigned) -1;
+                  new_range->last = (unsigned) -1;
+                  new_range->assem_name = xstrdup (func_name);
+                }
 	    }
 	  else
 	    {
@@ -677,15 +694,28 @@ enable_disable_pass (const char *arg, bo
           new_range->next = slot;
           VEC_replace (uid_range_p, *tab, pass->static_pass_number,
                        new_range);
-
           if (is_enable)
-            inform (UNKNOWN_LOCATION,
-                    "enable pass %s for functions in the range of [%u, %u]",
-                    phase_name, new_range->start, new_range->last);
+            {
+              if (new_range->assem_name)
+                inform (UNKNOWN_LOCATION,
+                        "enable pass %s for function %s",
+                        phase_name, new_range->assem_name);
+              else
+                inform (UNKNOWN_LOCATION,
+                        "enable pass %s for functions in the range of [%u, %u]",
+                        phase_name, new_range->start, new_range->last);
+            }
           else
-            inform (UNKNOWN_LOCATION,
-                    "disable pass %s for functions in the range of [%u, %u]",
-                    phase_name, new_range->start, new_range->last);
+            {
+              if (new_range->assem_name)
+                inform (UNKNOWN_LOCATION,
+                        "disable pass %s for function %s",
+                        phase_name, new_range->assem_name);
+              else
+                inform (UNKNOWN_LOCATION,
+                        "disable pass %s for functions in the range of [%u, %u]",
+                        phase_name, new_range->start, new_range->last);
+            }
 
 	  one_range = next_range;
 	} while (next_range);
@@ -719,6 +749,7 @@ is_pass_explicitly_enabled_or_disabled (
 {
   uid_range_p slot, range;
   int cgraph_uid;
+  const char *aname = NULL;
 
   if (!tab
       || (unsigned) pass->static_pass_number >= VEC_length (uid_range_p, tab)
@@ -730,6 +761,8 @@ is_pass_explicitly_enabled_or_disabled (
     return false;
 
   cgraph_uid = func ? cgraph_get_node (func)->uid : 0;
+  if (func && DECL_ASSEMBLER_NAME_SET_P (func))
+    aname = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (func));
 
   range = slot;
   while (range)
@@ -737,6 +770,9 @@ is_pass_explicitly_enabled_or_disabled (
       if ((unsigned) cgraph_uid >= range->start
 	  && (unsigned) cgraph_uid <= range->last)
 	return true;
+      if (range->assem_name && aname
+          && !strcmp (range->assem_name, aname))
+        return true;
       range = range->next;
     }