PATCH RFC: More control over which functions are instrumented

PATCH RFC: More control over which functions are instrumented

[Ian Lance Taylor]

We've had a -finstrument-functions option for a while, which inserts
calls to __cyg_profile_func_enter and __cyg_profile_func_exit at the
start and end of each function.  This can be handy, but it is rather
less handy in practice with C++.  The issue is simply that there are a
bunch of inline functions which it is uninteresting to instrument, and
these functions come from standard headers so you can't really avoid
compiling them with -finstrument-functions.

This patch was developed a while back by Yaz Saito at Google.  It adds
two new options: -finstrument-functions-exclude-file-list and
-finstrument-functions-exclude-function-list which give more control
over which functions are instrumented.

This patch is all middle-end, which is in my bailiwick, but I'd like
to see whether there are any comments on this approach.

Bootstrapped and tested on i686-pc-linux-gnu.

Ian


2007-08-01  Yaz Saito  <saito@google.com>
	    Ian Lance Taylor  <iant@google.com>

	* common.opt (finstrument-functions-exclude-function-list): New
	option.
	(finstrument-functions-exclude-file-list): New option.
	* opts.c (char_p): Define and DEF_VEC.
	(flag_instrument_functions_exclude_functions): New static
	variable.
	(flag_instrument_functions_exclude_files): New static variable.
	(add_instrument_functions_exclude_list): New static function.
	(flag_instrument_functions_exclude_p): New function.
	(common_handle_option): Handle new options.
	* flags.h (flag_instrument_functions_exclude_p): Declare.
	* gimplify.c (gimplify_function_tree): Call
	flag_instrument_functions_exclude_p.
	* doc/invoke.texi (Option Summary): Mention new options.
	(Code Gen Options): Document new options.


Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 127073)
+++ doc/invoke.texi	(working copy)
@@ -810,6 +810,8 @@ See S/390 and zSeries Options.
 -fnon-call-exceptions  -funwind-tables @gol
 -fasynchronous-unwind-tables @gol
 -finhibit-size-directive  -finstrument-functions @gol
+-finstrument-functions-exclude-function-list=@var{sym},@var{sym},@dots{} @gol
+-finstrument-functions-exclude-file-list=@var{file},@var{file},@dots{} @gol
 -fno-common  -fno-ident @gol
 -fpcc-struct-return  -fpic  -fPIC -fpie -fPIE @gol
 -fno-jump-tables @gol
@@ -14664,6 +14666,37 @@ interrupt routines, and any functions fr
 cannot safely be called (perhaps signal handlers, if the profiling
 routines generate output or allocate memory).
 
+@item -finstrument-functions-exclude-file-list=@var{file},@var{file},@dots{}
+@opindex finstrument-functions-exclude-file-list
+
+Set the list of functions that are excluded from instrumentation (see
+the description of @code{-finstrument-functions}).  If the file that
+contains a function definition matches with one of @var{file}, then
+that function is not instrumented.  The match is done on substrings:
+if the @var{file} parameter is a substring of the file name, it is
+considered to be a match.
+
+For example,
+@code{-finstrument-functions-exclude-file-list=/bits/stl,include/sys}
+will exclude any inline function defined in files whose pathnames
+contain @code{/bits/stl} or @code{include/sys}.
+
+If, for some reason, you want to include letter @code{','} in one of
+@var{sym}, write @code{'\,'}. For example,
+@code{-finstrument-functions-exclude-file-list='\,\,tmp'}
+(note the single quote surrounding the option).
+
+@item -finstrument-functions-exclude-function-list=@var{sym},@var{sym},@dots{}
+@opindex finstrument-functions-exclude-function-list
+
+This is similar to @code{-finstrument-functions-exclude-file-list},
+but this option sets the list of function names to be excluded from
+instrumentation.  The function name to be matched is its user-visible
+name, such as @code{vector<int> blah(const vector<int> &)}, not the
+internal mangled name (e.g., @code{_Z4blahRSt6vectorIiSaIiEE}).  The
+match is done on substrings: if the @var{sym} parameter is a substring
+of the function name, it is considered to be a match.
+
 @item -fstack-check
 @opindex fstack-check
 Generate code to verify that you do not go beyond the boundary of the
Index: flags.h
===================================================================
--- flags.h	(revision 127073)
+++ flags.h	(working copy)
@@ -281,6 +281,10 @@ extern bool flag_speculative_prefetching
 #define abi_version_at_least(N) \
   (flag_abi_version == 0 || flag_abi_version >= (N))
 
+/* Return whether the function should be excluded from
+   instrumentation.  */
+extern bool flag_instrument_functions_exclude_p (tree fndecl);
+
 /* True if the given mode has a NaN representation and the treatment of
    NaN operands is important.  Certain optimizations, such as folding
    x * 0 into 0, are not correct for NaN operands, and are normally
Index: opts.c
===================================================================
--- opts.c	(revision 127073)
+++ opts.c	(working copy)
@@ -353,6 +353,15 @@ static bool flag_unroll_loops_set, flag_
 static bool flag_value_profile_transformations_set;
 static bool flag_peel_loops_set, flag_branch_probabilities_set;
 
+/* Functions excluded from profiling.  */
+
+typedef char *char_p; /* For DEF_VEC_P.  */
+DEF_VEC_P(char_p);
+DEF_VEC_ALLOC_P(char_p,heap);
+
+static VEC(char_p,heap) *flag_instrument_functions_exclude_functions;
+static VEC(char_p,heap) *flag_instrument_functions_exclude_files;
+
 /* Input file names.  */
 const char **in_fnames;
 unsigned num_in_fnames;
@@ -602,6 +611,87 @@ add_input_filename (const char *filename
   in_fnames[num_in_fnames - 1] = filename;
 }
 
+/* Add functions or file names to a vector of names to exclude from
+   instrumentation.  */
+
+static void
+add_instrument_functions_exclude_list (VEC(char_p,heap) **pvec,
+				       const char* arg)
+{
+  char *tmp;
+  char *r;
+  char *w;
+  char *token_start;
+
+  /* We never free this string.  */
+  tmp = xstrdup (arg);
+
+  r = tmp;
+  w = tmp;
+  token_start = tmp;
+
+  while (*r != '\0')
+    {
+      if (*r == ',')
+	{
+	  *w++ = '\0';
+	  ++r;
+	  VEC_safe_push (char_p, heap, *pvec, token_start);
+	  token_start = w;
+	}
+      if (*r == '\\' && r[1] == ',')
+	{
+	  *w++ = ',';
+	  r += 2;
+	}
+      else
+	*w++ = *r++;
+    }
+  if (*token_start != '\0')
+    VEC_safe_push (char_p, heap, *pvec, token_start);
+}
+
+/* Return whether we should exclude FNDECL from instrumentation.  */
+
+bool
+flag_instrument_functions_exclude_p (tree fndecl)
+{
+  if (VEC_length (char_p, flag_instrument_functions_exclude_functions) > 0)
+    {
+      const char *name;
+      int i;
+      char *s;
+
+      name = lang_hooks.decl_printable_name (fndecl, 0);
+      for (i = 0;
+	   VEC_iterate (char_p, flag_instrument_functions_exclude_functions,
+			i, s);
+	   ++i)
+	{
+	  if (strstr (name, s) != NULL)
+	    return true;
+	}
+    }
+
+  if (VEC_length (char_p, flag_instrument_functions_exclude_files) > 0)
+    {
+      const char *name;
+      int i;
+      char *s;
+
+      name = DECL_SOURCE_FILE (fndecl);
+      for (i = 0;
+	   VEC_iterate (char_p, flag_instrument_functions_exclude_files, i, s);
+	   ++i)
+	{
+	  if (strstr (name, s) != NULL)
+	    return true;
+	}
+    }
+
+  return false;
+}
+
 /* Decode and handle the vector of command line options.  LANG_MASK
    contains has a single bit set representing the current
    language.  */
@@ -1474,6 +1564,16 @@ common_handle_option (size_t scode, cons
       set_param_value ("max-inline-insns-auto", value / 2);
       break;
 
+    case OPT_finstrument_functions_exclude_function_list_:
+      add_instrument_functions_exclude_list
+	(&flag_instrument_functions_exclude_functions, arg);
+      break;
+
+    case OPT_finstrument_functions_exclude_file_list_:
+      add_instrument_functions_exclude_list
+	(&flag_instrument_functions_exclude_files, arg);
+      break;
+
     case OPT_fmessage_length_:
       pp_set_line_maximum_length (global_dc->printer, value);
       break;
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 127073)
+++ gimplify.c	(working copy)
@@ -6494,7 +6494,8 @@ gimplify_function_tree (tree fndecl)
      catch the exit hook.  */
   /* ??? Add some way to ignore exceptions for this TFE.  */
   if (flag_instrument_function_entry_exit
-      && ! DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (fndecl))
+      && !DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (fndecl)
+      && !flag_instrument_functions_exclude_p (fndecl))
     {
       tree tf, x, bind;
 
Index: common.opt
===================================================================
--- common.opt	(revision 127073)
+++ common.opt	(working copy)
@@ -575,6 +575,14 @@ finstrument-functions
 Common Report Var(flag_instrument_function_entry_exit)
 Instrument function entry and exit with profiling calls
 
+finstrument-functions-exclude-function-list=
+Common RejectNegative Joined
+-finstrument-functions-exclude-function-list=name,...  Do not instrument listed functions
+
+finstrument-functions-exclude-file-list=
+Common RejectNegative Joined
+-finstrument-functions-exclude-file-list=filename,...  Do not instrument functions listed in files
+
 fipa-cp
 Common Report Var(flag_ipa_cp) Optimization
 Perform Interprocedural constant propagation

Re: PATCH RFC: More control over which functions are instrumented

[Andrew Pinski]

On 01 Aug 2007 13:21:25 -0700, Ian Lance Taylor <iant@google.com> wrote:
> We've had a -finstrument-functions option for a while, which inserts
> calls to __cyg_profile_func_enter and __cyg_profile_func_exit at the
> start and end of each function.  This can be handy, but it is rather
> less handy in practice with C++.  The issue is simply that there are a
> bunch of inline functions which it is uninteresting to instrument, and
> these functions come from standard headers so you can't really avoid
> compiling them with -finstrument-functions.

I rather have instrumentation done differently than it is now instead
of these options.  Just have an option to turn off instrumentation for
functions that get inlined (so the instrumentation happens after
inlining).  These options help but you still have to maintain a list
of functions which can get hard to maintain unlike instrumenting after
inlining.

Note I was not the first who thought of going this route and it was
requested in bug number 28205 (and I also got a request for that way
of doing instrumentation from a private bug report inside Sony).

Thanks,
Andrew Pinski

Re: PATCH RFC: More control over which functions are instrumented

[Ian Lance Taylor]

"Andrew Pinski" <pinskia@gmail.com> writes:

> On 01 Aug 2007 13:21:25 -0700, Ian Lance Taylor <iant@google.com> wrote:
> > We've had a -finstrument-functions option for a while, which inserts
> > calls to __cyg_profile_func_enter and __cyg_profile_func_exit at the
> > start and end of each function.  This can be handy, but it is rather
> > less handy in practice with C++.  The issue is simply that there are a
> > bunch of inline functions which it is uninteresting to instrument, and
> > these functions come from standard headers so you can't really avoid
> > compiling them with -finstrument-functions.
> 
> I rather have instrumentation done differently than it is now instead
> of these options.  Just have an option to turn off instrumentation for
> functions that get inlined (so the instrumentation happens after
> inlining).  These options help but you still have to maintain a list
> of functions which can get hard to maintain unlike instrumenting after
> inlining.

-finstrument-functions-exclude-function-list is there because it seems
odd to not have it, but the more commonly used option will be
-finstrument-functions-exclude-file-list.  Note that files are matched
as substrings, so you can say
-finstrument-functions-exclude-file-list=include/c++ to exclude any
function defined in a file in ...include/c++...

Turning off instrumentation for functions which are inlined is useful
but it is not the same.  It is reasonable to actually want
instrumentation for inlined functions that you wrote in an optimized
build.

That is, I see the ideas as independent; I don't see why one should
block the other.

Ian

Re: PATCH RFC: More control over which functions are instrumented

[Andrew Pinski]

On 01 Aug 2007 14:49:32 -0700, Ian Lance Taylor <iant@google.com> wrote:
> -finstrument-functions-exclude-function-list is there because it seems
> odd to not have it, but the more commonly used option will be
> -finstrument-functions-exclude-file-list.  Note that files are matched
> as substrings, so you can say
> -finstrument-functions-exclude-file-list=include/c++ to exclude any
> function defined in a file in ...include/c++...

Ick, basing on where the function is defined seems like a hack.  That
is like the non NO_IMPLICIT_EXTERN_C problem.  Maybe I see this as
less useful than the instrument vs inline function issue.  You never
know people might have an include/c++ directory in their source tree
too which includes their C++ headers.  This one of the reasons why I
think it is less useful.

Thanks,
Andrew Pinski

Re: PATCH RFC: More control over which functions are instrumented

[Ian Lance Taylor]

Ian Lance Taylor <iant@google.com> writes:

> This patch was developed a while back by Yaz Saito at Google.  It adds
> two new options: -finstrument-functions-exclude-file-list and
> -finstrument-functions-exclude-function-list which give more control
> over which functions are instrumented.

I have committed this patch.

I also added some simple test cases, as follows.

Ian


2007-08-07  Ian Lance Taylor  <iant@google.com>

	* gcc.dg/instrument-1.c: New test.
	* gcc.dg/instrument-2.c: New test.
	* gcc.dg/instrument-3.c: New test.


instrument-1.c:

/* { dg-do compile } */
/* { dg-options "-finstrument-functions" } */

void fn () { }

/* { dg-final { scan-assembler "__cyg_profile_func_enter" } } */
/* { dg-final { scan-assembler "__cyg_profile_func_exit" } } */


instrument-2.c:

/* { dg-do compile } */
/* { dg-options "-finstrument-functions -finstrument-functions-exclude-function-list=fn" } */

void fn () { }

/* { dg-final { scan-assembler-not "__cyg_profile_func_enter" } } */
/* { dg-final { scan-assembler-not "__cyg_profile_func_exit" } } */


instrument-3.c:

/* { dg-do compile } */
/* { dg-options "-finstrument-functions -finstrument-functions-exclude-file-list=instrument-3" } */

void fn () { }

/* { dg-final { scan-assembler-not "__cyg_profile_func_enter" } } */
/* { dg-final { scan-assembler-not "__cyg_profile_func_exit" } } */