[google] Add function name to function_patch_* sections (issue9025045)

2013-04-30 Thread Harshit Chopra
Adding function name to the function_patch_* sections when -ffunction-sections 
is provided. Helps in garbage collecting dead functions with the help of linker.

Tested:
  Tested using 'make -k check-gcc RUNTESTFLAGS=i386.exp=patch* 
--target_board=unix\{-m32,,-m64\}'.

2013-04-30  Harshit Chopra  hars...@google.com

* gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue):
(ix86_elf_asm_named_section):

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index aa6ec82..7cb832b 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11285,6 +11285,8 @@ ix86_output_function_nops_prologue_epilogue (FILE *file,
   unsigned int section_flags = SECTION_RELRO;
   char *section_name_comdat = NULL;
   const char *decl_section_name = NULL;
+  const char *func_name = NULL;
+  char *section_name_function_sections = NULL;
   size_t len;
 
   gcc_assert (num_remaining_nops = 0);
@@ -11336,12 +11338,24 @@ ix86_output_function_nops_prologue_epilogue (FILE 
*file,
 {
   decl_section_name =
   TREE_STRING_POINTER (DECL_SECTION_NAME (current_function_decl));
-  len = strlen (decl_section_name) + strlen (section_name) + 1;
+  len = strlen (decl_section_name) + strlen (section_name) + 2;
   section_name_comdat = (char *) alloca (len);
   sprintf (section_name_comdat, %s.%s, section_name, decl_section_name);
   section_name = section_name_comdat;
   section_flags |= SECTION_LINKONCE;
 }
+  else if (flag_function_sections)
+{
+  func_name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
+  if (func_name)
+{
+  len = strlen (func_name) + strlen (section_name) + 2;
+  section_name_function_sections = (char *) alloca (len);
+  sprintf (section_name_function_sections, %s.%s, section_name,
+   func_name);
+  section_name = section_name_function_sections;
+}
+}
   section = get_section (section_name, section_flags, current_function_decl);
   switch_to_section (section);
   /* Align the section to 8-byte boundary.  */
@@ -11369,7 +11383,7 @@ ix86_elf_asm_named_section (const char *name, unsigned 
int flags,
 tree decl)
 {
   const char *section_name = name;
-  if (HAVE_COMDAT_GROUP  flags  SECTION_LINKONCE)
+  if (!flag_function_sections  HAVE_COMDAT_GROUP  flags  SECTION_LINKONCE)
 {
   const int prologue_section_name_length =
   sizeof(FUNCTION_PATCH_PROLOGUE_SECTION) - 1;

--
This patch is available for review at http://codereview.appspot.com/9025045


[google] Port revisions for -mpatch-functions-for-instrumentation option back to google-main. (issue7301068)

2013-02-08 Thread Harshit Chopra
2013-02-08  Harshit Chopra hars...@google.com

Porting revisions r183548, r183888, r186118, r192231, r193381.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index ab416ff..04b973f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -396,6 +396,13 @@ static tree ignore_attribute (tree *, tree, tree, int, 
bool *);
 static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool *);
 static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
 
+static tree handle_always_patch_for_instrumentation_attribute (tree *, tree,
+   tree, int,
+   bool *);
+static tree handle_never_patch_for_instrumentation_attribute (tree *, tree,
+  tree, int,
+  bool *);
+
 static void check_function_nonnull (tree, int, tree *);
 static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
 static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT);
@@ -804,6 +811,13 @@ const struct attribute_spec c_common_attribute_table[] =
  The name contains space to prevent its usage in source code.  */
   { fn spec,   1, 1, false, true, true,
  handle_fnspec_attribute, false },
+  { always_patch_for_instrumentation, 0, 0, true,  false, false,
+  
handle_always_patch_for_instrumentation_attribute,
+  false },
+  { never_patch_for_instrumentation, 0, 0, true,  false, false,
+  handle_never_patch_for_instrumentation_attribute,
+  false },
+
   { NULL, 0, 0, false, false, false, NULL, false }
 };
 
@@ -9158,6 +9172,48 @@ handle_no_thread_safety_analysis_attribute (tree *node, 
tree name,
   return NULL_TREE;
 }
 
+/* Handle a always_patch_for_instrumentation attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_always_patch_for_instrumentation_attribute (tree *node, tree name,
+   tree ARG_UNUSED (args),
+   int ARG_UNUSED (flags),
+   bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) == FUNCTION_DECL)
+{
+  /* Disable inlining if forced instrumentation.  */
+  DECL_UNINLINABLE (*node) = 1;
+}
+  else
+{
+  warning (OPT_Wattributes, %qE attribute ignored, name);
+  *no_add_attrs = true;
+}
+  return NULL_TREE;
+}
+
+
+/* Handle a never_patch_for_instrumentation attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_never_patch_for_instrumentation_attribute (tree *node, tree name,
+  tree ARG_UNUSED (args),
+  int ARG_UNUSED (flags),
+  bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) != FUNCTION_DECL)
+{
+  warning (OPT_Wattributes, %qE attribute ignored, name);
+  *no_add_attrs = true;
+}
+  return NULL_TREE;
+}
+
+
+
 /* Check for valid arguments being passed to a function with FNTYPE.
There are NARGS arguments in the array ARGARRAY.  */
 void
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6972ae6..4e0d770 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10968,6 +10968,13 @@ ix86_expand_epilogue (int style)
returns.  */
 static bool patch_current_function_p = false;
 
+static inline bool
+has_attribute (const char* attribute_name)
+{
+  return lookup_attribute (attribute_name,
+   DECL_ATTRIBUTES (current_function_decl)) != NULL;
+}
+
 /* Return true if we patch the current function. By default a function
is patched if it has loops or if the number of insns is greater than
patch_functions_min_instructions (number of insns roughly translates
@@ -10983,6 +10990,13 @@ check_should_patch_current_function (void)
   int num_loops = 0;
   int min_functions_instructions;
 
+  /* If a function has an attribute forcing patching on or off, do as it
+ indicates.  */
+  if (has_attribute (always_patch_for_instrumentation))
+return true;
+  else if (has_attribute (never_patch_for_instrumentation))
+return false;
+
   /* Patch the function if it has at least a loop.  */
   if (!patch_functions_ignore_loops)
 {
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c
new file mode 100644
index 000..cad6f2d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-options

Re: [google] Add attributes: always_patch_for_instrumentation and never_patch_for_instrumentation (issue6821051)

2012-11-06 Thread Harshit Chopra
Yes, will do, but probably not so soon. Once I have some spare time to
prepare my case for this being useful to public.

Meanwhile, this patch is just for google-main and then I will port it
to google_4-7 and adds to the already existing functionality of
-mpatch-function-for-instrumentation.

Thanks,
Harshit


On Mon, Nov 5, 2012 at 12:29 PM, Xinliang David Li davi...@google.com wrote:
 It does not hurt to submit the patch for review -- you need to provide
 more background and motivation for this work
 1) comparison with -finstrument-functions (runtime overhead etc)
 2) use model difference (production binary ..)
 3) Interesting examples of use cases (with graphs).

 thanks,

 David

 On Mon, Nov 5, 2012 at 12:20 PM, Harshit Chopra hars...@google.com wrote:
 Thanks David for the review. My comments are inline.


 On Sat, Nov 3, 2012 at 12:38 PM, Xinliang David Li davi...@google.com 
 wrote:

 Harshit, Nov 5 is the gcc48 cutoff date. If you want to have the x-ray
 instrumentation feature into this release, you will need to port your
 patch and submit for trunk review now.


 I am a bit too late now, I guess. If I target for the next release,
 will it create any issues for the gcc48 release?




 On Tue, Oct 30, 2012 at 5:15 PM, Harshit Chopra hars...@google.com wrote:
  Adding function attributes: 'always_patch_for_instrumentation' and 
  'never_patch_for_instrumentation' to always patch a function or to never 
  patch a function, respectively, when given the option 
  -mpatch-functions-for-instrumentation. Additionally, the attribute 
  always_patch_for_instrumentation disables inlining of that function.
 
  Tested:
Tested by 'crosstool-validate.py --crosstool_ver=16 --testers=crosstool'
 
  ChangeLog:
 
  2012-10-30  Harshit Chopra hars...@google.com
 
  * gcc/c-family/c-common.c 
  (handle_always_patch_for_instrumentation_attribute): Handle
always_patch_for_instrumentation attribute and turn inlining off for 
  the function.
  (handle_never_patch_for_instrumentation_attribute): Handle 
  never_patch_for_instrumentation
attribute of a function.
  * gcc/config/i386/i386.c (check_should_patch_current_function): 
  Takes into account
always_patch_for_instrumentation or never_patch_for_instrumentation 
  attribute when
deciding that a function should be patched.
  * 
  gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c: Test 
  case
to test for never_patch_for_instrumentation attribute.
  * gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c: 
  Test case to
test for always_patch_for_instrumentation attribute.
  * gcc/tree.h (struct GTY): Add fields for the two attributes and 
  macros to access
the fields.
  diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
  index ab416ff..998645d 100644
  --- a/gcc/c-family/c-common.c
  +++ b/gcc/c-family/c-common.c
  @@ -396,6 +396,13 @@ static tree ignore_attribute (tree *, tree, tree, 
  int, bool *);
   static tree handle_no_split_stack_attribute (tree *, tree, tree, int, 
  bool *);
   static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
 
  +static tree handle_always_patch_for_instrumentation_attribute (tree *, 
  tree,
  +   tree, int,
  +   bool *);

 Move bool * to the previous line.


 If I do that, it goes beyond the 80 char boundary.



  +static tree handle_never_patch_for_instrumentation_attribute (tree *, 
  tree,
  +  tree, int,
  +  bool *);
  +

 Same here.


 As above.



   static void check_function_nonnull (tree, int, tree *);
   static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
   static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT);
  @@ -804,6 +811,13 @@ const struct attribute_spec 
  c_common_attribute_table[] =
The name contains space to prevent its usage in source code.  */
 { fn spec,   1, 1, false, true, true,
handle_fnspec_attribute, false },
  +  { always_patch_for_instrumentation, 0, 0, true,  false, false,
  +  
  handle_always_patch_for_instrumentation_attribute,
  +  false },
  +  { never_patch_for_instrumentation, 0, 0, true,  false, false,
  +  
  handle_never_patch_for_instrumentation_attribute,
  +  false },
  +
 { NULL, 0, 0, false, false, false, NULL, false }
   };
 
  @@ -9158,6 +9172,48 @@ handle_no_thread_safety_analysis_attribute (tree 
  *node, tree name,
 return NULL_TREE;
   }
 
  +/* Handle a always_patch_for_instrumentation attribute; arguments as in
  +   struct attribute_spec.handler.  */

 Add new line here


 Done.



  +static

[google] Add attributes: always_patch_for_instrumentation and never_patch_for_instrumentation (issue6821051)

2012-11-05 Thread Harshit Chopra
2012-11-05  Harshit Chopra  hars...@google.com

* gcc/c-family/c-common.c 
(handle_always_patch_for_instrumentation_attribute): Handle
  always_patch_for_instrumentation attribute and turn inlining off for the 
function.
(handle_never_patch_for_instrumentation_attribute): Handle
  never_patch_for_instrumentation attribute of a function.
* gcc/config/i386/i386.c (has_attribute): Checks the presence of an 
attribute.
(check_should_patch_current_function): Takes into account
  always_patch_for_instrumentation or never_patch_for_instrumentation attribute
  when deciding that a function should be patched.
* gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c 
(int main): Test case
  to test for never_patch_for_instrumentation attribute.
* gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c: Test 
case to test for
  always_patch_for_instrumentation attribute.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index ab416ff..04b973f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -396,6 +396,13 @@ static tree ignore_attribute (tree *, tree, tree, int, 
bool *);
 static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool *);
 static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
 
+static tree handle_always_patch_for_instrumentation_attribute (tree *, tree,
+   tree, int,
+   bool *);
+static tree handle_never_patch_for_instrumentation_attribute (tree *, tree,
+  tree, int,
+  bool *);
+
 static void check_function_nonnull (tree, int, tree *);
 static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
 static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT);
@@ -804,6 +811,13 @@ const struct attribute_spec c_common_attribute_table[] =
  The name contains space to prevent its usage in source code.  */
   { fn spec,   1, 1, false, true, true,
  handle_fnspec_attribute, false },
+  { always_patch_for_instrumentation, 0, 0, true,  false, false,
+  
handle_always_patch_for_instrumentation_attribute,
+  false },
+  { never_patch_for_instrumentation, 0, 0, true,  false, false,
+  handle_never_patch_for_instrumentation_attribute,
+  false },
+
   { NULL, 0, 0, false, false, false, NULL, false }
 };
 
@@ -9158,6 +9172,48 @@ handle_no_thread_safety_analysis_attribute (tree *node, 
tree name,
   return NULL_TREE;
 }
 
+/* Handle a always_patch_for_instrumentation attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_always_patch_for_instrumentation_attribute (tree *node, tree name,
+   tree ARG_UNUSED (args),
+   int ARG_UNUSED (flags),
+   bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) == FUNCTION_DECL)
+{
+  /* Disable inlining if forced instrumentation.  */
+  DECL_UNINLINABLE (*node) = 1;
+}
+  else
+{
+  warning (OPT_Wattributes, %qE attribute ignored, name);
+  *no_add_attrs = true;
+}
+  return NULL_TREE;
+}
+
+
+/* Handle a never_patch_for_instrumentation attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_never_patch_for_instrumentation_attribute (tree *node, tree name,
+  tree ARG_UNUSED (args),
+  int ARG_UNUSED (flags),
+  bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) != FUNCTION_DECL)
+{
+  warning (OPT_Wattributes, %qE attribute ignored, name);
+  *no_add_attrs = true;
+}
+  return NULL_TREE;
+}
+
+
+
 /* Check for valid arguments being passed to a function with FNTYPE.
There are NARGS arguments in the array ARGARRAY.  */
 void
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6972ae6..4e0d770 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10968,6 +10968,13 @@ ix86_expand_epilogue (int style)
returns.  */
 static bool patch_current_function_p = false;
 
+static inline bool
+has_attribute (const char* attribute_name)
+{
+  return lookup_attribute (attribute_name,
+   DECL_ATTRIBUTES (current_function_decl)) != NULL;
+}
+
 /* Return true if we patch the current function. By default a function
is patched if it has loops or if the number of insns is greater than
patch_functions_min_instructions (number of insns roughly translates
@@ -10983,6 +10990,13

Re: [google] Add attributes: always_patch_for_instrumentation and never_patch_for_instrumentation (issue6821051)

2012-11-05 Thread Harshit Chopra
Thanks David for the review. My comments are inline.


On Sat, Nov 3, 2012 at 12:38 PM, Xinliang David Li davi...@google.com wrote:

 Harshit, Nov 5 is the gcc48 cutoff date. If you want to have the x-ray
 instrumentation feature into this release, you will need to port your
 patch and submit for trunk review now.


I am a bit too late now, I guess. If I target for the next release,
will it create any issues for the gcc48 release?




 On Tue, Oct 30, 2012 at 5:15 PM, Harshit Chopra hars...@google.com wrote:
  Adding function attributes: 'always_patch_for_instrumentation' and 
  'never_patch_for_instrumentation' to always patch a function or to never 
  patch a function, respectively, when given the option 
  -mpatch-functions-for-instrumentation. Additionally, the attribute 
  always_patch_for_instrumentation disables inlining of that function.
 
  Tested:
Tested by 'crosstool-validate.py --crosstool_ver=16 --testers=crosstool'
 
  ChangeLog:
 
  2012-10-30  Harshit Chopra hars...@google.com
 
  * gcc/c-family/c-common.c 
  (handle_always_patch_for_instrumentation_attribute): Handle
always_patch_for_instrumentation attribute and turn inlining off for the 
  function.
  (handle_never_patch_for_instrumentation_attribute): Handle 
  never_patch_for_instrumentation
attribute of a function.
  * gcc/config/i386/i386.c (check_should_patch_current_function): 
  Takes into account
always_patch_for_instrumentation or never_patch_for_instrumentation 
  attribute when
deciding that a function should be patched.
  * 
  gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c: Test case
to test for never_patch_for_instrumentation attribute.
  * gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c: 
  Test case to
test for always_patch_for_instrumentation attribute.
  * gcc/tree.h (struct GTY): Add fields for the two attributes and 
  macros to access
the fields.
  diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
  index ab416ff..998645d 100644
  --- a/gcc/c-family/c-common.c
  +++ b/gcc/c-family/c-common.c
  @@ -396,6 +396,13 @@ static tree ignore_attribute (tree *, tree, tree, int, 
  bool *);
   static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool 
  *);
   static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
 
  +static tree handle_always_patch_for_instrumentation_attribute (tree *, 
  tree,
  +   tree, int,
  +   bool *);

 Move bool * to the previous line.


If I do that, it goes beyond the 80 char boundary.



  +static tree handle_never_patch_for_instrumentation_attribute (tree *, tree,
  +  tree, int,
  +  bool *);
  +

 Same here.


As above.



   static void check_function_nonnull (tree, int, tree *);
   static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
   static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT);
  @@ -804,6 +811,13 @@ const struct attribute_spec c_common_attribute_table[] 
  =
The name contains space to prevent its usage in source code.  */
 { fn spec,   1, 1, false, true, true,
handle_fnspec_attribute, false },
  +  { always_patch_for_instrumentation, 0, 0, true,  false, false,
  +  
  handle_always_patch_for_instrumentation_attribute,
  +  false },
  +  { never_patch_for_instrumentation, 0, 0, true,  false, false,
  +  
  handle_never_patch_for_instrumentation_attribute,
  +  false },
  +
 { NULL, 0, 0, false, false, false, NULL, false }
   };
 
  @@ -9158,6 +9172,48 @@ handle_no_thread_safety_analysis_attribute (tree 
  *node, tree name,
 return NULL_TREE;
   }
 
  +/* Handle a always_patch_for_instrumentation attribute; arguments as in
  +   struct attribute_spec.handler.  */

 Add new line here


Done.



  +static tree
  +handle_always_patch_for_instrumentation_attribute (tree *node, tree name,
  +   tree ARG_UNUSED (args),
  +   int ARG_UNUSED (flags),
  +   bool *no_add_attrs)
  +{
  +  if (TREE_CODE (*node) == FUNCTION_DECL)
  +{
  +  DECL_FORCE_PATCHING_FOR_INSTRUMENTATION (*node) = 1;
  +  DECL_UNINLINABLE (*node) = 1;
  +}
  +  else
  +{
  +  warning (OPT_Wattributes, %qE attribute ignored, name);
  +  *no_add_attrs = true;
  +}
  +  return NULL_TREE;
  +}
  +
  +
  +/* Handle a never_patch_for_instrumentation attribute; arguments as in
  +   struct attribute_spec.handler.  */

 A new line here.


Done

[google] Add attributes: always_patch_for_instrumentation and never_patch_for_instrumentation (issue6821051)

2012-10-30 Thread Harshit Chopra
Adding function attributes: 'always_patch_for_instrumentation' and 
'never_patch_for_instrumentation' to always patch a function or to never patch 
a function, respectively, when given the option 
-mpatch-functions-for-instrumentation. Additionally, the attribute 
always_patch_for_instrumentation disables inlining of that function.

Tested:
  Tested by 'crosstool-validate.py --crosstool_ver=16 --testers=crosstool'

ChangeLog:

2012-10-30  Harshit Chopra hars...@google.com

* gcc/c-family/c-common.c 
(handle_always_patch_for_instrumentation_attribute): Handle
  always_patch_for_instrumentation attribute and turn inlining off for the 
function.
(handle_never_patch_for_instrumentation_attribute): Handle 
never_patch_for_instrumentation
  attribute of a function.
* gcc/config/i386/i386.c (check_should_patch_current_function): Takes 
into account
  always_patch_for_instrumentation or never_patch_for_instrumentation attribute 
when
  deciding that a function should be patched.
* gcc/testsuite/gcc.target/i386/patch-functions-force-no-patching.c: 
Test case
  to test for never_patch_for_instrumentation attribute.
* gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c: Test 
case to
  test for always_patch_for_instrumentation attribute.
* gcc/tree.h (struct GTY): Add fields for the two attributes and macros 
to access
  the fields.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index ab416ff..998645d 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -396,6 +396,13 @@ static tree ignore_attribute (tree *, tree, tree, int, 
bool *);
 static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool *);
 static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
 
+static tree handle_always_patch_for_instrumentation_attribute (tree *, tree,
+   tree, int,
+   bool *);
+static tree handle_never_patch_for_instrumentation_attribute (tree *, tree,
+  tree, int,
+  bool *);
+
 static void check_function_nonnull (tree, int, tree *);
 static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
 static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT);
@@ -804,6 +811,13 @@ const struct attribute_spec c_common_attribute_table[] =
  The name contains space to prevent its usage in source code.  */
   { fn spec,   1, 1, false, true, true,
  handle_fnspec_attribute, false },
+  { always_patch_for_instrumentation, 0, 0, true,  false, false,
+  
handle_always_patch_for_instrumentation_attribute,
+  false },
+  { never_patch_for_instrumentation, 0, 0, true,  false, false,
+  handle_never_patch_for_instrumentation_attribute,
+  false },
+
   { NULL, 0, 0, false, false, false, NULL, false }
 };
 
@@ -9158,6 +9172,48 @@ handle_no_thread_safety_analysis_attribute (tree *node, 
tree name,
   return NULL_TREE;
 }
 
+/* Handle a always_patch_for_instrumentation attribute; arguments as in
+   struct attribute_spec.handler.  */
+static tree
+handle_always_patch_for_instrumentation_attribute (tree *node, tree name,
+   tree ARG_UNUSED (args),
+   int ARG_UNUSED (flags),
+   bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) == FUNCTION_DECL)
+{
+  DECL_FORCE_PATCHING_FOR_INSTRUMENTATION (*node) = 1;
+  DECL_UNINLINABLE (*node) = 1;
+}
+  else
+{
+  warning (OPT_Wattributes, %qE attribute ignored, name);
+  *no_add_attrs = true;
+}
+  return NULL_TREE;
+}
+
+
+/* Handle a never_patch_for_instrumentation attribute; arguments as in
+   struct attribute_spec.handler.  */
+static tree
+handle_never_patch_for_instrumentation_attribute (tree *node, tree name,
+  tree ARG_UNUSED (args),
+  int ARG_UNUSED (flags),
+  bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) == FUNCTION_DECL)
+DECL_FORCE_NO_PATCHING_FOR_INSTRUMENTATION (*node) = 1;
+  else
+{
+  warning (OPT_Wattributes, %qE attribute ignored, name);
+  *no_add_attrs = true;
+}
+  return NULL_TREE;
+}
+
+
+
 /* Check for valid arguments being passed to a function with FNTYPE.
There are NARGS arguments in the array ARGARRAY.  */
 void
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6972ae6..b1475bf 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10983,6 +10983,13

[google] Emit relative addresses to function patch sections instead of absolute addresses. (issue6572065)

2012-09-28 Thread Harshit Chopra
commit fc3a55ccec9bc770c79f8a221f5abd397befc8f6
Author: Harshit Chopra hars...@google.com
Date:   Thu Sep 20 17:49:59 2012 -0700

Instead of emitting absolute addresses to the function patch sections, emit 
relative addresses. Absolute addresses might require relocation, which is time 
consuming and fraught with other issues.

M   gcc/config/i386/i386.c

Tested:
  Ran make check-gcc and manually confirmed that the affected tests pass.

ChangeLog:

2012-09-28  Harshit Chopra  hars...@google.com

* gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue): 
Emit relative address to function patch sections.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index f72b0b5..8c9334f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11098,7 +11098,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file,
$LFPEL0:
  pre_instruction
  0x90 (repeated num_actual_nops times)
- .quad $LFPESL0
+ .quad $LFPESL0 - .
  followed by section 'section_name' which contains the address
  of instruction at 'label'.
*/
@@ -0,7 +0,10 @@ ix86_output_function_nops_prologue_epilogue (FILE 
*file,
 asm_fprintf (file, ASM_BYTE0x90\n);
 
   fprintf (file, ASM_QUAD);
+  /* Output section_label - . for the relative address of the entry in
+ the section 'section_name'.  */
   assemble_name_raw (file, section_label);
+  fprintf (file,  - .);
   fprintf (file, \n);
 
   /* Emit the backpointer section. For functions belonging to comdat group,
@@ -11144,7 +11147,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file,
  .quad $LFPEL0
*/
   ASM_OUTPUT_INTERNAL_LABEL (file, section_label);
-  fprintf(file, ASM_QUAD\t);
+  fprintf(file, ASM_QUAD);
   assemble_name_raw (file, label);
   fprintf (file, \n);
 

--
This patch is available for review at http://codereview.appspot.com/6572065


[google] Minor cleanup and test fixes for -mpatch-functions-for-instrumentation. (issue5877043)

2012-03-21 Thread Harshit Chopra
2012-03-21   Harshit Chopra  hars...@google.com

  Minor changes:
i386.c: made check_should_patch_current_function C90 compatible.
i386.md: Added '\t' to bytes generated by
 ix86_output_function_nops_prologue_epilogue for proper formatting
 of assembly.
patch-functions-*.c: Fixed verification in tests. Added a test to verify
 nop-bytes generated for sibling calls and another test
 to verify a binary with nop-bytes runs properly.

* gcc/config/i386/i386.c (check_should_patch_current_function):
* gcc/config/i386/i386.md:
* gcc/testsuite/gcc.target/i386/patch-functions-1.c (void foo):
(int main):
* gcc/testsuite/gcc.target/i386/patch-functions-2.c:
* gcc/testsuite/gcc.target/i386/patch-functions-3.c:
* gcc/testsuite/gcc.target/i386/patch-functions-4.c:
* gcc/testsuite/gcc.target/i386/patch-functions-5.c:
* gcc/testsuite/gcc.target/i386/patch-functions-6.c:
* gcc/testsuite/gcc.target/i386/patch-functions-7.c:
* gcc/testsuite/gcc.target/i386/patch-functions-8.c (int foo):
(int bar):
* gcc/testsuite/gcc.target/i386/patch-functions-sibling-call.c:

Testing method:
  make check-gcc RUNTESTFLAGS=i386.exp=patch-functions* 
--target_board=\unix{-m32,}\

Patch to be applied to google/main.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 08bd5f0..be1f7a4 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10981,6 +10981,7 @@ check_should_patch_current_function (void)
   const char* func_name = NULL;
   struct loops loops;
   int num_loops = 0;
+  int min_functions_instructions;
 
   /* Patch the function if it has at least a loop.  */
   if (!patch_functions_ignore_loops)
@@ -11007,7 +11008,7 @@ check_should_patch_current_function (void)
   strcmp(main, func_name) == 0)
 return true;
 
-  int min_functions_instructions =
+  min_functions_instructions =
   PARAM_VALUE (PARAM_FUNCTION_PATCH_MIN_INSTRUCTIONS);
   if (min_functions_instructions  0)
 {
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 08353ff..38a04ae 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11688,7 +11688,7 @@
   /* Emit 10 nop bytes after ret.  */
   if (ix86_output_function_nops_prologue_epilogue (asm_out_file,
   
FUNCTION_PATCH_EPILOGUE_SECTION,
-  ret,
+  \tret,
   10))
return ;
 }
@@ -11712,7 +11712,7 @@
   /* Emit 9 nop bytes after rep;ret.  */
   if (ix86_output_function_nops_prologue_epilogue (asm_out_file,
   
FUNCTION_PATCH_EPILOGUE_SECTION,
-  rep\;ret,
+  \trep\;ret,
   9))
return ;
 }
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
index 308e8c3..aa1f424 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
@@ -1,5 +1,5 @@
 /* Verify -mpatch-functions-for-instrumentation works.  */
-/* { dg-do run } */
+/* { dg-do compile } */
 /* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation } */
 
@@ -8,13 +8,16 @@
 /* Check nop-bytes at end.  */
 /* { dg-final { scan-assembler ret(.*).byte\t0x90(.*).byte\t0x90 } } */
 
-void foo() {
+__attribute__ ((noinline))
+void foo()
+{
   /* Dummy loop.  */
   int x = 0;
   while (++x);
 }
 
-int main() {
+int main()
+{
   foo();
   return 0;
 }
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
index 6baad32..78de867 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
@@ -1,4 +1,4 @@
-/* { dg-do run } */
+/* { dg-do compile } */
 /* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation 
-mno-patch-functions-main-always } */
 
@@ -8,11 +8,14 @@
 /* { dg-final { scan-assembler-not .byte\t0xeb,0x09(.*).byte\t0x90 } } */
 /* { dg-final { scan-assembler-not ret(.*).byte\t0x90(.*).byte\t0x90 } } */
 
-void foo() {
+__attribute__ ((noinline))
+void foo()
+{
   int x = 0;
 }
 
-int main() {
+int main()
+{
   foo();
   return 0;
 }
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
index 49b57a8..9e8eb52 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
@@ -1,4 +1,4 @@
-/* { dg-do run } */
+/* { dg-do compile

[google] Fixing function patching tests (issue5626049)

2012-02-03 Thread Harshit Chopra
Fixes the following tests by restricting them to 64-bit target environment.

Tested: Using 'make -k check-gcc RUNTESTFLAGS=i386.exp=patch* 
--target_board=unix\{-m32,,-m64\}' and crosstool-validate.py script.

Patch to be applied to google/main branch.

2012-02-03   Harshit Chopra  hars...@google.com

  Restricting the tests to run for 64-bit target since function patching
  is only supported for 64-bit targets.

* gcc/testsuite/gcc.target/i386/patch-functions-1.c:
* gcc/testsuite/gcc.target/i386/patch-functions-2.c:
* gcc/testsuite/gcc.target/i386/patch-functions-3.c:
* gcc/testsuite/gcc.target/i386/patch-functions-4.c:
* gcc/testsuite/gcc.target/i386/patch-functions-5.c:
* gcc/testsuite/gcc.target/i386/patch-functions-6.c:
* gcc/testsuite/gcc.target/i386/patch-functions-7.c:

diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
index 1d88d45..308e8c3 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
@@ -1,5 +1,6 @@
 /* Verify -mpatch-functions-for-instrumentation works.  */
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation } */
 
 /* Check nop-bytes at beginning.  */
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
index ed6e7ce..6baad32 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation 
-mno-patch-functions-main-always } */
 
 /* Function is small to be instrumented with default values. Check there
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
index d7449c5..49b57a8 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation --param 
function-patch-min-instructions=0 } */
 
 /* Function should have nop-bytes with -mpatch-function-min-instructions=0.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
index f554d92..5fcbd0f 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-4.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation 
-mpatch-functions-ignore-loops -mno-patch-functions-main-always } */
 
 /* Function is too small to be patched when ignoring the loop.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
index bd1dafb..1f9cccf 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-5.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation 
-mpatch-functions-ignore-loops --param function-patch-min-instructions=0 } */
 
 /* Function should be patched with nop bytes with given options.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
index 46b715d..5bf844f 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-6.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation } */
 
 /* 'main' function should always be patched, irrespective of how small it is.
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c 
b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
index adeda59..e2c50a0 100644
--- a/gcc/testsuite/gcc.target/i386/patch-functions-7.c
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
@@ -1,4 +1,5 @@
-/* { dg-do run} */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
 /* { dg-options -mpatch-functions-for-instrumentation 
-mno-patch-functions-main-always } */
 
 /* 'main' shouldn't be patched with the option 
-mno-patch-functions-main-always.

--
This patch is available for review at http://codereview.appspot.com/5626049


Re: [google] Fixing function patching tests (issue5626049)

2012-02-03 Thread Harshit Chopra
Thanks for the review. Committed to google-main.

--
Harshit



On Fri, Feb 3, 2012 at 4:33 PM, Xinliang David Li davi...@google.com wrote:
 ok for google branches.

 David

 On Fri, Feb 3, 2012 at 3:20 PM, Harshit Chopra hars...@google.com wrote:
 Fixes the following tests by restricting them to 64-bit target environment.

 Tested: Using 'make -k check-gcc RUNTESTFLAGS=i386.exp=patch* 
 --target_board=unix\{-m32,,-m64\}' and crosstool-validate.py script.

 Patch to be applied to google/main branch.

 2012-02-03   Harshit Chopra  hars...@google.com

  Restricting the tests to run for 64-bit target since function patching
  is only supported for 64-bit targets.

        * gcc/testsuite/gcc.target/i386/patch-functions-1.c:
        * gcc/testsuite/gcc.target/i386/patch-functions-2.c:
        * gcc/testsuite/gcc.target/i386/patch-functions-3.c:
        * gcc/testsuite/gcc.target/i386/patch-functions-4.c:
        * gcc/testsuite/gcc.target/i386/patch-functions-5.c:
        * gcc/testsuite/gcc.target/i386/patch-functions-6.c:
        * gcc/testsuite/gcc.target/i386/patch-functions-7.c:

 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
 index 1d88d45..308e8c3 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
 @@ -1,5 +1,6 @@
  /* Verify -mpatch-functions-for-instrumentation works.  */
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation } */

  /* Check nop-bytes at beginning.  */
 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
 index ed6e7ce..6baad32 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
 @@ -1,4 +1,5 @@
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation 
 -mno-patch-functions-main-always } */

  /* Function is small to be instrumented with default values. Check there
 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
 index d7449c5..49b57a8 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
 @@ -1,4 +1,5 @@
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation --param 
 function-patch-min-instructions=0 } */

  /* Function should have nop-bytes with -mpatch-function-min-instructions=0.
 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
 index f554d92..5fcbd0f 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-4.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
 @@ -1,4 +1,5 @@
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation 
 -mpatch-functions-ignore-loops -mno-patch-functions-main-always } */

  /* Function is too small to be patched when ignoring the loop.
 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
 index bd1dafb..1f9cccf 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-5.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
 @@ -1,4 +1,5 @@
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation 
 -mpatch-functions-ignore-loops --param function-patch-min-instructions=0 } 
 */

  /* Function should be patched with nop bytes with given options.
 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
 index 46b715d..5bf844f 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-6.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
 @@ -1,4 +1,5 @@
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation } */

  /* 'main' function should always be patched, irrespective of how small it 
 is.
 diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c 
 b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
 index adeda59..e2c50a0 100644
 --- a/gcc/testsuite/gcc.target/i386/patch-functions-7.c
 +++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
 @@ -1,4 +1,5 @@
 -/* { dg-do run} */
 +/* { dg-do run } */
 +/* { dg-require-effective-target lp64 } */
  /* { dg-options -mpatch-functions-for-instrumentation 
 -mno-patch-functions-main-always } */

  /* 'main' shouldn't be patched with the option 
 -mno-patch-functions-main-always.

 --
 This patch is available for review at http://codereview.appspot.com/5626049


MAINTAINERS file diff after adding myself

2012-01-23 Thread Harshit Chopra
Index: MAINTAINERS
===
--- MAINTAINERS (revision 183449)
+++ MAINTAINERS (working copy)
@@ -336,6 +336,7 @@
 Gabriel Charette                               gch...@google.com
 Chandra Chavva                                 ccha...@redhat.com
 Fabien Ch�ne                                   fab...@gcc.gnu.org
+Harshit Chopra        hars...@google.com
 William Cohen                                  wco...@redhat.com
 Josh Conner                                    jcon...@apple.com
 R. Kelley Cook                                 kc...@gcc.gnu.org


Re: [google] Patch to enable efficient function level instrumentation

2011-11-28 Thread Harshit Chopra
Ping!

--
Harshit


Patch to enable efficient function level instrumentation (issue5416043)

2011-11-17 Thread Harshit Chopra
From f63ec02c0a720174489fe450b3cc43eb00fd4bdd Mon Sep 17 00:00:00 2001
From: Harshit Chopra hars...@google.com
Date: Thu, 3 Nov 2011 17:29:23 -0700
Subject: [PATCH] Mechanism to provide efficient function instrumentation 
(x86-64)

Summary: This patch aims at providing an efficient way to instrument a binary 
at function level for x86-64 architecture. Existing mechanisms (like 
-finstrument-functions) are expensive, especially when not instrumenting, which 
calls for the need of separate binaries for deployment and debugging for long 
running systems. This patch aims to remove this need of two separate binaries.

Detail: This patch adds a mechanism to insert a sequence of Nop bytes at 
function entry and at function exits, which are used to instrument a binary at 
function level by manipulating these nops at runtime. Emission of these nop 
bytes is controlled by the flag -mpatch-functions-for-instrumentation and other 
related flags. This patch only adds support for binaries for x86-64 
architecture.

For each function in the binary, 11-bytes of nops are added at the entry and 
10-bytes of nops at exit points (just after the return). For functions which 
make a tail call to another function, 11-bytes of nops are added just before 
the tail call jump. These nops can be patched at runtime to call appropriate 
instrumentation routines for function entry and exit.

The format of 11 bytes of nops (at function prologue and before tail call jump) 
is:
L0: jmp L1
XX
.quad L2

.section function_patch_section_name
L2: .quad L0

.text
L1: ... (function code)

The section 'function_patch_section_name' is used to store a back pointer to 
these nops. If the nops belong to function prologue, the name of this section 
is _function_patch_prologue, or else if the nops belong to function exit, the 
name is _function_patch_epilogue.

The 'XX' bytes can be anything (0x00-0xff) since they are dead bytes and act as 
fillers.

Similarly at function exit (just after ret instruction), the format of the 
10-bytes of nops is:
...
L0: ret
XX
XX
.quad L1

.section _function_patch_epilogue
L1: .quad L0

In order to handle functions belonging to COMDAT group, the name of the 
function patch section is 
function_patch_section_name.function_decl_section. Such sections are 
later renamed to function_patch_section_name just before emitting the 
assembly by the routine ix86_elf_asm_named_section().

Conceptually, these nops can be replaced by the following instructions at 
runtime:
  mov func_id, %r10d
  call InstrumentationFunction

The 'mov' being a 6 byte instruction and the call being a 5-byte instruction, a 
total of 11-bytes are needed for patching. Hence, the need for 11-bytes of 
nops. 10-bytes of nops are needed after the 'ret' instruction since the return 
is also patched and the call replaced by a jump to InstrumentationFunction 
(similar to a tail call).

To control emission of these nops globally and on a per-function basis 
(somewhat), the following flags are provided:
* -mpatch-functions-for-instrumentation: Master flag to control emission of 
these nops. The rest of the flags below have no effect if this flag is not 
provided.
* -mpatch-functions-min-instructions: If provided, only those functions having 
number of instructions greater than this flag value will have these nops at 
prologue and exit. Default is 200.
* -mpatch-functions-ignore-loops: To ignore loops when deciding whether to have 
these nops for a function. By default, functions having loops always include 
these nops.
* -mno-patch-functions-main-always: The 'main' function always has these nops 
to have the ability to rely on a function to call initialize instrumentation 
code. This flag treats main function as any other function.


Some issues with this patch:
* This patch prohibits garbage collection of dead functions at link time since 
there are pointers from the sections _function_patch_prologue and 
_function_patch_epilogue to the dead function.
* The existence of random data bytes in the dead code part of these nops in the 
middle of text section confuses disassembling tools like objdump.

Suggestions/comments for this patch are welcome. Also, suggestions on dealing 
with the above issues will be really helpful.


Testing done: Wrote tests to test code generation for the different cases and 
then ran 'make -k check-gcc'

Patch to be applied to google/main and trunk.

---
 gcc/config/i386/i386-protos.h |4 +
 gcc/config/i386/i386.c|  199 +
 gcc/config/i386/i386.md   |   26 +++-
 gcc/config/i386/i386.opt  |   16 ++
 gcc/testsuite/gcc.target/i386/patch-functions-1.c |   14 ++
 gcc/testsuite/gcc.target/i386/patch-functions-2.c |   12 ++
 gcc/testsuite/gcc.target/i386/patch-functions-3.c |   12 ++
 gcc/testsuite/gcc.target/i386