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

login
register
mail settings
Submitter Harshit Chopra
Date Feb. 9, 2013, 2:57 a.m.
Message ID <20130209025730.D6FCD121033@hchopra.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/219361/
State New
Headers show

Comments

Harshit Chopra - Feb. 9, 2013, 2:57 a.m.
2013-02-08  Harshit Chopra <harshit@google.com>

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


--
This patch is available for review at http://codereview.appspot.com/7301068
Xinliang David Li - Feb. 9, 2013, 3 a.m.
ok.

thanks,

David

On Fri, Feb 8, 2013 at 6:57 PM, Harshit Chopra <harshit@google.com> wrote:
> 2013-02-08  Harshit Chopra <harshit@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 0000000..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 "-mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */
> +
> +/* Even complicated functions shouldn't get patched if they have the
> +   never_patch_for_instrumentation attribute. */
> +
> +/* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
> +/* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
> +
> +__attribute__ ((never_patch_for_instrumentation))
> +int foo () {
> +  volatile unsigned x = 0;
> +  volatile unsigned y = 1;
> +  x += y;
> +  x *= y;
> +  while (++x)
> +    foo ();
> +  return y;
> +}
> +
> +
> +int main ()
> +{
> +  int x = 0;
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c b/gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c
> new file mode 100644
> index 0000000..86ad159
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target lp64 } */
> +/* { dg-options "-O3 -mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */
> +
> +/* Functions which have the always_patch attribute should be patched no matter
> +   what.  Check that there are nop-bytes at the beginning and end of the
> +   function.  We add -O3 so that the compiler will try to inline foo (but it
> +   will be blocked by the attribute).  */
> +
> +/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
> +/* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
> +
> +__attribute__ ((always_patch_for_instrumentation))
> +static int foo () {
> +  return 3;
> +}
> +
> +int main () {
> +  volatile int x = foo ();
> +}
>
> --
> This patch is available for review at http://codereview.appspot.com/7301068

Patch

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 0000000..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 "-mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */
+
+/* Even complicated functions shouldn't get patched if they have the
+   never_patch_for_instrumentation attribute. */
+
+/* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+__attribute__ ((never_patch_for_instrumentation))
+int foo () {
+  volatile unsigned x = 0;
+  volatile unsigned y = 1;
+  x += y;
+  x *= y;
+  while (++x)
+    foo ();
+  return y;
+}
+
+
+int main ()
+{
+  int x = 0;
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c b/gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c
new file mode 100644
index 0000000..86ad159
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-force-patching.c
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-options "-O3 -mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */
+
+/* Functions which have the always_patch attribute should be patched no matter
+   what.  Check that there are nop-bytes at the beginning and end of the
+   function.  We add -O3 so that the compiler will try to inline foo (but it
+   will be blocked by the attribute).  */
+
+/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+__attribute__ ((always_patch_for_instrumentation))
+static int foo () {
+  return 3;
+}
+
+int main () {
+  volatile int x = foo ();
+}