From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30593 invoked by alias); 31 Oct 2012 00:15:43 -0000 Received: (qmail 30277 invoked by uid 22791); 31 Oct 2012 00:15:40 -0000 X-SWARE-Spam-Status: No, hits=-4.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-vb0-f73.google.com (HELO mail-vb0-f73.google.com) (209.85.212.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 31 Oct 2012 00:15:33 +0000 Received: by mail-vb0-f73.google.com with SMTP id fs19so100944vbb.2 for ; Tue, 30 Oct 2012 17:15:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:to:subject:user-agent:mime-version:content-type :content-transfer-encoding:message-id:from:x-gm-message-state; bh=uhXQnBlYUj9Fccn5oAs61R4ybXHF3Y7Pd3RoKhgK5uw=; b=U2tZ1F+UKYhdztV3GSwBfGQSoP+o3d8lHpwEUvLS4iXHOG3ZG4oiEjA9cO/l57xGNM j1KgWfnrCxnjM2yGFmVejxVQ5nQ39KC3W3oh3QJ7my1aGtWFwU1JwSReVeQUwL3vaV6b ICIQtwPV4YirBq5fVkSgMQHM5rHRfsabAZxutPehexNTgm9uP05F73jKkLMFeapgJC0W 0vsco6CsATQ7M7J3cJ7zaVt153NNaEXKbgsP0MC0aKw7pbTIk3BvEM5UspuKuVkkCKOP F4pMcC7vFxWRsmDlOv3gvuUwsANgqGvEvXjBHIutCcmrnQ4CR0W7vpohsU21lI/00z5+ 3HyQ== Received: by 10.236.179.72 with SMTP id g48mr23906983yhm.37.1351642531981; Tue, 30 Oct 2012 17:15:31 -0700 (PDT) Received: from wpzn3.hot.corp.google.com (216-239-44-65.google.com [216.239.44.65]) by gmr-mx.google.com with ESMTPS id r10si100958ann.1.2012.10.30.17.15.31 (version=TLSv1/SSLv3 cipher=AES128-SHA); Tue, 30 Oct 2012 17:15:31 -0700 (PDT) Received: from hchopra.mtv.corp.google.com (hchopra.mtv.corp.google.com [172.17.133.27]) by wpzn3.hot.corp.google.com (Postfix) with ESMTP id BBF55100047; Tue, 30 Oct 2012 17:15:31 -0700 (PDT) Received: by hchopra.mtv.corp.google.com (Postfix, from userid 48577) id 5D197121AAA; Tue, 30 Oct 2012 17:15:31 -0700 (PDT) Date: Wed, 31 Oct 2012 00:39:00 -0000 To: gcc-patches@gcc.gnu.org, davidxl@google.com, reply@codereview.appspotmail.com Subject: [google] Add attributes: always_patch_for_instrumentation and never_patch_for_instrumentation (issue6821051) User-Agent: Heirloom mailx 12.5 6/20/10 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-Id: <20121031001531.5D197121AAA@hchopra.mtv.corp.google.com> From: harshit@google.com (Harshit Chopra) X-Gm-Message-State: ALoCoQknFWtzs/2NhMlDquHBjo5Z6lobqlr5A6AE44t4SQb7jvLD+4aVsiFNNZHMfTW55wDJBIDBJtMkzztcxft5ZzTUzhHbo2OucZ7I9SLPWqwemnx/NOItlNGDHdag7xevx0HzPNlDnJUkf8WcVW8JF6CC9hJzrRk22+zP7LcSVm38CirkNTTWBVytbD3YdIkYz5mc662O Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-10/txt/msg02841.txt.bz2 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 * 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 @@ 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 (DECL_FORCE_PATCHING_FOR_INSTRUMENTATION (current_function_decl)) + return true; + else if (DECL_FORCE_NO_PATCHING_FOR_INSTRUMENTATION (current_function_decl)) + 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 (); +} diff --git a/gcc/tree.h b/gcc/tree.h index 1eafaa0..9d26a00 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -3474,6 +3474,16 @@ struct GTY(()) #define DECL_NO_INLINE_WARNING_P(NODE) \ (FUNCTION_DECL_CHECK (NODE)->function_decl.no_inline_warning_flag) +/* In a FUNCTION_DECL, nonzero if patching is forced. */ +#define DECL_FORCE_PATCHING_FOR_INSTRUMENTATION(NODE) \ + (FUNCTION_DECL_CHECK (NODE)->function_decl.force_patching_for_instrumentation) + +/* In a FUNCTION_DECL, nonzero if not patching is forced. */ +#define DECL_FORCE_NO_PATCHING_FOR_INSTRUMENTATION(NODE)\ + (FUNCTION_DECL_CHECK (NODE)->function_decl.force_no_patching_for_instrumentation) + + + /* Nonzero if a FUNCTION_CODE is a TM load/store. */ #define BUILTIN_TM_LOAD_STORE_P(FN) \ ((FN) >= BUILT_IN_TM_STORE_1 && (FN) <= BUILT_IN_TM_LOAD_RFW_LDOUBLE) @@ -3586,7 +3596,8 @@ struct GTY(()) tree_function_decl { unsigned has_debug_args_flag : 1; unsigned tm_clone_flag : 1; - /* 1 bit left */ + unsigned force_patching_for_instrumentation : 1; + unsigned force_no_patching_for_instrumentation : 1; }; /* The source language of the translation-unit. */ -- This patch is available for review at http://codereview.appspot.com/6821051