From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17481 invoked by alias); 12 Oct 2015 10:56:42 -0000 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 Received: (qmail 17468 invoked by uid 89); 12 Oct 2015 10:56:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mx07-00178001.pphosted.com Received: from mx07-00178001.pphosted.com (HELO mx07-00178001.pphosted.com) (62.209.51.94) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 12 Oct 2015 10:56:39 +0000 Received: from pps.filterd (m0046668.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.14.5/8.14.5) with SMTP id t9CAo4o7027958; Mon, 12 Oct 2015 12:56:34 +0200 Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 1xedaubumn-1 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Mon, 12 Oct 2015 12:56:34 +0200 Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 0E94734; Mon, 12 Oct 2015 10:56:11 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas4.st.com [10.75.90.69]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 96FD8ABAC; Mon, 12 Oct 2015 10:56:31 +0000 (GMT) Received: from [164.129.122.197] (164.129.122.197) by webmail-eu.st.com (10.75.90.13) with Microsoft SMTP Server (TLS) id 8.3.389.2; Mon, 12 Oct 2015 12:56:31 +0200 From: Christian Bruel Subject: Re: [PATCH ARM]: PR67745: Fix function alignment after __attribute__ 2/2 To: Bernd Schmidt , "law@redhat.com" References: <560A90F2.5010708@st.com> <560C31CD.7060009@redhat.com> <560CDCD7.9080108@st.com> <560D5B36.2020600@st.com> <5614C412.5080400@st.com> <5614F17B.5060402@redhat.com> <5614F7D0.8010409@st.com> <56155841.6000404@redhat.com> <56155B72.4020807@redhat.com> <56166C36.7040600@st.com> <56166DA5.3030909@redhat.com> <561674C1.8030008@st.com> <56167E09.3010502@redhat.com> CC: "gcc-patches@gcc.gnu.org" , "Ramana.Radhakrishnan@arm.com" , "kyrylo.tkachov@arm.com" , "richard.earnshaw@arm.com" X-No-Archive: yes Message-ID: <561B91DE.5040909@st.com> Date: Mon, 12 Oct 2015 10:56:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <56167E09.3010502@redhat.com> Content-Type: multipart/mixed; boundary="------------040500020208060901040104" X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-10-12_04:2015-10-09,2015-10-12,1970-01-01 signatures=0 X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg01113.txt.bz2 --------------040500020208060901040104 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1445 On 10/08/2015 04:30 PM, Bernd Schmidt wrote: > On 10/08/2015 03:50 PM, Christian Bruel wrote: >> Humm, I don't know what kind of alignment optimization for functions we >> have based on a declaration only. greping DECL_ALIGN on functions there >> are some bits in the ipa-icf code that seems to merge code using this >> information, but I think we have a definition at that point. >> but honestly, I'm very unfamiliar with this pass. Do you have something >> else in mind ? > > I had a vague memory of us optimizing that, but I can't find the code > either and maybe it's just not there. That doesn't mean someone isn't > going to add it in the future, and I'm uncomfortable leaving incorrect > DECL_ALIGN values around. > > It looks like rest_of_decl_compilation may be a good place to take care > of declarations, but using FUNCTION_BOUNDARY is probably going to give > the wrong results. So maybe a target hook, function_boundary_for_decl, > defaulting to just returning FUNCTION_BOUNDARY? Eventually it could > replace the macro entirely. > > yes I see, I was hoping to avoid a new hook, but as you said it seems mandatory for the mere declaration case. Here is one proposal, it defaults to nothing and the ARM implementation does not need to handle the vptr bit setting. so that simplifies a lot the things. The hook is called from rest_of_decl_compilation for mere declarations and allocate_struct_function for definitions. --------------040500020208060901040104 Content-Type: text/x-patch; name="align_hook.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="align_hook.patch" Content-length: 7362 2015-09-29 Christian Bruel PR target/67745 * config/arm/arm.h (FUNCTION_BOUNDARY): Use FUNCTION_BOUNDARY_P. (FUNCTION_BOUNDARY_P): New macro: * config/arm/arm.c (TARGET_RELAYOUT_FUNCTION, arm_relayout_function): New hook. * doc/tm.texi.in (TARGET_RELAYOUT_FUNCTION): Document. * doc/tm.texi (TARGET_RELAYOUT_FUNCTION): New hook. * gcc/target.def (TARGET_RELAYOUT_FUNCTION): Likewise. * gcc/function.c (allocate_struct_function): Call relayout_function hook. * gcc/passes.c (rest_of_decl_compilation): Likewise. * gcc/targhooks.c (default_relayout_function): New function. * gcc/targhooks.h (default_relayout_function): Declare. --- gnu_trunk.p0/gcc/gcc/config/arm/arm.c 2015-10-12 10:34:27.599740376 +0200 +++ gnu_trunk.devs/gcc/gcc/config/arm/arm.c 2015-10-12 12:26:51.607398021 +0200 @@ -250,6 +250,7 @@ static void arm_override_options_after_c static void arm_option_print (FILE *, int, struct cl_target_option *); static void arm_set_current_function (tree); static bool arm_can_inline_p (tree, tree); +static void arm_relayout_function (tree); static bool arm_valid_target_attribute_p (tree, tree, tree, int); static unsigned HOST_WIDE_INT arm_shift_truncation_mask (machine_mode); static bool arm_macro_fusion_p (void); @@ -405,6 +406,9 @@ static const struct attribute_spec arm_a #undef TARGET_CAN_INLINE_P #define TARGET_CAN_INLINE_P arm_can_inline_p +#undef TARGET_RELAYOUT_FUNCTION +#define TARGET_RELAYOUT_FUNCTION arm_relayout_function + #undef TARGET_OPTION_OVERRIDE #define TARGET_OPTION_OVERRIDE arm_option_override @@ -29825,6 +29829,23 @@ arm_can_inline_p (tree caller ATTRIBUTE_ return true; } +/* Hook to fix function's alignment affected by target attribute. */ + +static void +arm_relayout_function (tree fndecl) +{ + if (DECL_USER_ALIGN (fndecl)) + return; + + tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (fndecl); + + if (!callee_tree) + callee_tree = target_option_default_node; + + DECL_ALIGN (fndecl) = + FUNCTION_BOUNDARY_P (TREE_TARGET_OPTION (callee_tree)->x_target_flags); +} + /* Inner function to process the attribute((target(...))), take an argument and set the current options from the argument. If we have a list, recursively go over the list. */ diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/config/arm/arm.h gnu_trunk.devs/gcc/gcc/config/arm/arm.h --- gnu_trunk.p0/gcc/gcc/config/arm/arm.h 2015-10-12 10:34:27.607740391 +0200 +++ gnu_trunk.devs/gcc/gcc/config/arm/arm.h 2015-10-12 12:27:55.507546958 +0200 @@ -565,7 +565,8 @@ extern int arm_arch_crc; #define PREFERRED_STACK_BOUNDARY \ (arm_abi == ARM_ABI_ATPCS ? 64 : STACK_BOUNDARY) -#define FUNCTION_BOUNDARY (TARGET_THUMB ? 16 : 32) +#define FUNCTION_BOUNDARY_P(flags) (TARGET_THUMB_P (flags) ? 16 : 32) +#define FUNCTION_BOUNDARY (FUNCTION_BOUNDARY_P (target_flags)) /* The lowest bit is used to indicate Thumb-mode functions, so the vbit must go into the delta field of pointers to member diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/doc/tm.texi gnu_trunk.devs/gcc/gcc/doc/tm.texi --- gnu_trunk.p0/gcc/gcc/doc/tm.texi 2015-10-12 10:33:29.907630642 +0200 +++ gnu_trunk.devs/gcc/gcc/doc/tm.texi 2015-10-12 12:33:33.880332253 +0200 @@ -9985,6 +9985,10 @@ default, inlining is not allowed if the specific target options and the caller does not use the same options. @end deftypefn +@deftypefn {Target Hook} void TARGET_RELAYOUT_FUNCTION (tree @var{fndecl}) +This target hook fixes function @var{fndecl} after attributes are processed. Default does nothing. On ARM, the default function's alignment is updated with the attribute target. +@end deftypefn + @node Emulated TLS @section Emulating TLS @cindex Emulated TLS diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/doc/tm.texi.in gnu_trunk.devs/gcc/gcc/doc/tm.texi.in --- gnu_trunk.p0/gcc/gcc/doc/tm.texi.in 2015-10-12 10:33:29.919630666 +0200 +++ gnu_trunk.devs/gcc/gcc/doc/tm.texi.in 2015-10-12 11:28:16.350590629 +0200 @@ -7274,6 +7274,8 @@ on this implementation detail. @hook TARGET_CAN_INLINE_P +@hook TARGET_RELAYOUT_FUNCTION + @node Emulated TLS @section Emulating TLS @cindex Emulated TLS diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/function.c gnu_trunk.devs/gcc/gcc/function.c --- gnu_trunk.p0/gcc/gcc/function.c 2015-10-12 10:33:32.715635978 +0200 +++ gnu_trunk.devs/gcc/gcc/function.c 2015-10-12 10:04:42.764482359 +0200 @@ -4840,6 +4840,9 @@ allocate_struct_function (tree fndecl, b for (tree parm = DECL_ARGUMENTS (fndecl); parm; parm = DECL_CHAIN (parm)) relayout_decl (parm); + + /* Similarly relayout the function decl. */ + targetm.target_option.relayout_function (fndecl); } if (!abstract_p && aggregate_value_p (result, fndecl)) --- gnu_trunk.p0/gcc/gcc/passes.c 2015-10-05 14:22:31.082400039 +0200 +++ gnu_trunk.devs/gcc/gcc/passes.c 2015-10-12 11:31:55.343150983 +0200 @@ -253,6 +253,11 @@ rest_of_decl_compilation (tree decl, } #endif + /* Now that we have activated any function-specific attributes + that might affect function decl, particularly align, relayout it. */ + if (TREE_CODE (decl) == FUNCTION_DECL) + targetm.target_option.relayout_function (decl); + timevar_pop (TV_VARCONST); } else if (TREE_CODE (decl) == TYPE_DECL diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/target.def gnu_trunk.devs/gcc/gcc/target.def --- gnu_trunk.p0/gcc/gcc/target.def 2015-10-05 14:22:20.142374948 +0200 +++ gnu_trunk.devs/gcc/gcc/target.def 2015-10-12 12:34:23.744447530 +0200 @@ -5620,6 +5620,12 @@ specific target options and the caller d bool, (tree caller, tree callee), default_target_can_inline_p) +DEFHOOK +(relayout_function, +"This target hook fixes function @var{fndecl} after attributes are processed. Default does nothing. On ARM, the default function's alignment is updated with the attribute target.", + void, (tree fndecl), + default_relayout_function) + HOOK_VECTOR_END (target_option) /* For targets that need to mark extra registers as live on entry to diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/targhooks.c gnu_trunk.devs/gcc/gcc/targhooks.c --- gnu_trunk.p0/gcc/gcc/targhooks.c 2015-08-25 10:28:00.504946542 +0200 +++ gnu_trunk.devs/gcc/gcc/targhooks.c 2015-10-12 11:24:50.866063124 +0200 @@ -1311,6 +1311,11 @@ default_target_option_pragma_parse (tree return false; } +void +default_relayout_function (tree fndecl) +{ +} + bool default_target_can_inline_p (tree caller, tree callee) { diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/targhooks.h gnu_trunk.devs/gcc/gcc/targhooks.h --- gnu_trunk.p0/gcc/gcc/targhooks.h 2015-10-05 14:22:17.774369515 +0200 +++ gnu_trunk.devs/gcc/gcc/targhooks.h 2015-10-12 11:24:40.914042025 +0200 @@ -161,6 +161,7 @@ extern bool default_hard_regno_scratch_o extern bool default_mode_dependent_address_p (const_rtx, addr_space_t); extern bool default_target_option_valid_attribute_p (tree, tree, tree, int); extern bool default_target_option_pragma_parse (tree, tree); +extern void default_relayout_function (tree); extern bool default_target_can_inline_p (tree, tree); extern bool default_valid_pointer_mode (machine_mode); extern bool default_ref_may_alias_errno (struct ao_ref *); --------------040500020208060901040104--