From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 78560 invoked by alias); 1 Jun 2015 11:30:12 -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 78541 invoked by uid 89); 1 Jun 2015 11:30:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 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, 01 Jun 2015 11:30:10 +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 t51BS5Lw006755; Mon, 1 Jun 2015 13:30:02 +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 1ur1guc92f-1 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Mon, 01 Jun 2015 13:30:02 +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 427CE38; Mon, 1 Jun 2015 11:29:58 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas3.st.com [10.75.90.18]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 22DF026CA; Mon, 1 Jun 2015 11:29:58 +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.342.0; Mon, 1 Jun 2015 13:29:57 +0200 Message-ID: <556C4235.5060203@st.com> Date: Mon, 01 Jun 2015 11:30:00 -0000 From: Christian Bruel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Kyrill Tkachov , Ramana Radhakrishnan , Sandra Loosemore CC: "gcc-patches@gcc.gnu.org" , Richard Earnshaw , "nickc@redhat.com" Subject: Re: [PATCH, ARM] attribute target (thumb,arm) [4/6] respin (5th) References: <554A243B.8010902@st.com> <554A9DB1.1070009@codesourcery.com> <554C7459.4000806@arm.com> <55599F54.4040807@st.com> <556C366B.8000203@arm.com> In-Reply-To: <556C366B.8000203@arm.com> X-No-Archive: yes Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-06-01_02:2015-05-29,2015-06-01,1970-01-01 signatures=0 X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00047.txt.bz2 hi Kyrill On 06/01/2015 12:39 PM, Kyrill Tkachov wrote: > > On 18/05/15 09:14, Christian Bruel wrote: >> Hi, > > Hi Christian, > A couple comments inline. > Overall, the approach looks ok to me, though I think we'll have to > generalise arm_valid_target_attribute_rec in the future if we want > to allow other target attributes. > the other fpu target attributes will be part of another set of developments, specific parsing strings will be added as they are implemented. > + > +/* Establish appropriate back-end context for processing the function > + FNDECL. The argument might be NULL to indicate processing at top > + level, outside of any function scope. */ > +static void > +arm_set_current_function (tree fndecl) > +{ > + if (!fndecl || fndecl == arm_previous_fndecl) > + return; > + > + tree old_tree = (arm_previous_fndecl > + ? DECL_FUNCTION_SPECIFIC_TARGET (arm_previous_fndecl) > + : NULL_TREE); > + > + tree new_tree = DECL_FUNCTION_SPECIFIC_TARGET (fndecl); > + > + arm_previous_fndecl = fndecl; > + if (old_tree == new_tree) > + ; > + > + else if (new_tree) > + { > + cl_target_option_restore (&global_options, > + TREE_TARGET_OPTION (new_tree)); > + > + if (TREE_TARGET_GLOBALS (new_tree)) > + restore_target_globals (TREE_TARGET_GLOBALS (new_tree)); > + else > + TREE_TARGET_GLOBALS (new_tree) > + = save_target_globals_default_opts (); > + } > + > + else if (old_tree) > + { > + new_tree = target_option_current_node; > + > + cl_target_option_restore (&global_options, > + TREE_TARGET_OPTION (new_tree)); > + if (TREE_TARGET_GLOBALS (new_tree)) > + restore_target_globals (TREE_TARGET_GLOBALS (new_tree)); > + else if (new_tree == target_option_default_node) > + restore_target_globals (&default_target_globals); > + else > + TREE_TARGET_GLOBALS (new_tree) > + = save_target_globals_default_opts (); > + } > + > + arm_option_params_internal (&global_options); > > > I thought the more common approach was to define TARGET_OPTION_RESTORE > that was supposed to restore the backend state, including calling arm_option_params_internal? > That way, cl_target_option_restore would do all that needs to be done to restore the backend. > TARGET_OPTION_RESTORE is fine to restore target-specific information from struct cl_target_option. Other global states might as well be expressed within set_current_function (e.g indeed I might use TARGET_OPTION_RESTORE to switch arm_fpu_attr in the fpu neon attribute). But IMHO arm_option_params_internal are fine to be called there since the 2 params only depend from x_target_flags without the need of a new macro.