From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45527 invoked by alias); 1 Jun 2015 12:55:34 -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 45517 invoked by uid 89); 1 Jun 2015 12:55:34 -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 12:55:32 +0000 Received: from pps.filterd (m0046670.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.14.5/8.14.5) with SMTP id t51Co582030083; Mon, 1 Jun 2015 14:55:25 +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 1uqekn7gjx-1 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Mon, 01 Jun 2015 14:55:25 +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 46C3138; Mon, 1 Jun 2015 12:55:23 +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 DB6BF297C; Mon, 1 Jun 2015 12:55:22 +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 14:55:22 +0200 Message-ID: <556C5639.4080203@st.com> Date: Mon, 01 Jun 2015 12:55: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> <556C4235.5060203@st.com> <556C530B.8000105@arm.com> In-Reply-To: <556C530B.8000105@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_03:2015-05-29,2015-06-01,1970-01-01 signatures=0 X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00068.txt.bz2 On 06/01/2015 02:41 PM, Kyrill Tkachov wrote: > > On 01/06/15 12:29, Christian Bruel wrote: >> 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. > > Ok, so you plan on working on fpu attributes as well? I have a prototype for fpu=neon, it works locally but there are still a few corner cases and testing to sort out before sending a draft. There are so many architectural variants to check that I might ask for help once it is a little bit more robust, and some might be similar with aarch64's simd. > >> >>> + >>> +/* 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. > > Ok, I see. > The patch looks ok to me modulo the typo nits I pointed out, but I think Ramana > should have the final say here as he's already started reviewing it and it adds quite > a lot of functionality. OK thanks, I'll wait for Ramana's final words. Cheers Christian > > Thanks, > Kyrill > >> >> >