From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103582 invoked by alias); 13 Oct 2015 08:03:08 -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 103572 invoked by uid 89); 13 Oct 2015 08:03:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.1 required=5.0 tests=AWL,BAYES_99,BAYES_999,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Oct 2015 08:03:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6A7813C; Tue, 13 Oct 2015 01:03:02 -0700 (PDT) Received: from [10.2.206.27] (e105545-lin.cambridge.arm.com [10.2.206.27]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4C65C3F21A; Tue, 13 Oct 2015 01:03:03 -0700 (PDT) Subject: Re: [PATCH ARM]: PR67745: Fix function alignment after __attribute__ 2/2 To: Christian Bruel , 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> <561B91DE.5040909@st.com> Cc: "gcc-patches@gcc.gnu.org" , "Ramana.Radhakrishnan@arm.com" , "kyrylo.tkachov@arm.com" , "richard.earnshaw@arm.com" From: Ramana Radhakrishnan Message-ID: <561CBAB7.7080104@foss.arm.com> Date: Tue, 13 Oct 2015 08:03: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: <561B91DE.5040909@st.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg01193.txt.bz2 > > 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. I'm not sure we have testsuite coverage for this - can you add a test or 2 ? > > > > > > > > align_hook.patch > > > 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. The ARM changes look ok to me. Please as usual watch out for any regressions. > * 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) Can this just be hook_void_tree ? Then you don't need the empty 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 *); >