* [google] Port revisions for -mpatch-functions-for-instrumentation option back to google-main. (issue7301068)
@ 2013-02-09 2:57 Harshit Chopra
2013-02-09 3:00 ` Xinliang David Li
0 siblings, 1 reply; 2+ messages in thread
From: Harshit Chopra @ 2013-02-09 2:57 UTC (permalink / raw)
To: gcc-patches, davidxl, reply
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [google] Port revisions for -mpatch-functions-for-instrumentation option back to google-main. (issue7301068)
2013-02-09 2:57 [google] Port revisions for -mpatch-functions-for-instrumentation option back to google-main. (issue7301068) Harshit Chopra
@ 2013-02-09 3:00 ` Xinliang David Li
0 siblings, 0 replies; 2+ messages in thread
From: Xinliang David Li @ 2013-02-09 3:00 UTC (permalink / raw)
To: Harshit Chopra; +Cc: GCC Patches, reply
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-02-09 3:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-09 2:57 [google] Port revisions for -mpatch-functions-for-instrumentation option back to google-main. (issue7301068) Harshit Chopra
2013-02-09 3:00 ` Xinliang David Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).