This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, ARM] attribute target (thumb,arm) [4/6] respin (5th)
- From: Christian Bruel <christian dot bruel at st dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>, Sandra Loosemore <sandra at codesourcery dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, "nickc at redhat dot com" <nickc at redhat dot com>
- Date: Mon, 1 Jun 2015 13:29:57 +0200
- Subject: Re: [PATCH, ARM] attribute target (thumb,arm) [4/6] respin (5th)
- Authentication-results: sourceware.org; auth=none
- References: <554A243B dot 8010902 at st dot com> <554A9DB1 dot 1070009 at codesourcery dot com> <554C7459 dot 4000806 at arm dot com> <55599F54 dot 4040807 at st dot com> <556C366B dot 8000203 at arm dot com>
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.