On Wed, Jan 8, 2014 at 1:45 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Jan 08, 2014 at 12:32:59PM +0100, Richard Biener wrote:
>> > Either before writing PCH c-common.c could call some tree.c routine that
>> > would traverse the cl_option_hash_table hash table and for every
>> > TARGET_OPTION_NODE in the hash table clear TREE_TARGET_GLOBALS.
>> > Or perhaps some gengtype extension to run some routine before PCH saving
>> > on the tree_target_option structs and clear the globals field in there.
>> > Or use GTY((user)) on tree_target_option, but then dunno how we'd handle
>> > the
>> > marking of the embedded opts field (and common).
>> > Any ideas?
>>
>> Well, a GTY((skip_pch)) would probably work. Or move the thing
>> out-of GC land (thus make cl_option_hash_table persistant) and
>> simply GTY((skip)) the pointer completely. Not sure if we ever
>> collect from it.
>
> Even if the pointer was out of GCC land and GTY((skip)), we'd need to clear
> it somewhere during PCH saving, as the containing structure is GC allocated.
>
> I've already implemented in the mean time the variant with the
> htab_traverse, all still reachable TARGET_OPTION_NODE trees should be in that
> hash table.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (in both cases with
> --enable-checking=yes,rtl and --enable-checking=release, for the
> i686-linux/release checking I had to fix an unrelated compare debug issue
> I'll post when I manage to reduce testcase).
>
> I'd like to get rid of all the XCNEW calls in target-globals.c as a
> follow-up.
>
> As for performance, for --enable-checking=release from very rough check
> on make -j48 bootstrap and make -j48 check times the patch is compile time
> neutral, on e.g. declare-simd-1.C testcase g++ is twice as fast with the
> patch though (~ 0.8 sec without the patch, ~ 0.3 sec with the patch, both
> for x86_64 and i686).
>
> Ok for trunk?
Works for me. Wait a bit for others to comment though.
Thanks,
Richard.
> 2014-01-07 Jakub Jelinek <ja...@redhat.com>
>
> PR target/58115
> * tree-core.h (struct target_globals): New forward declaration.
> (struct tree_target_option): Add globals field.
> * tree.h (TREE_TARGET_GLOBALS): Define.
> (prepare_target_option_nodes_for_pch): New prototype.
> * target-globals.h (struct target_globals): Define even if
> !SWITCHABLE_TARGET.
> * tree.c (prepare_target_option_node_for_pch,
> prepare_target_option_nodes_for_pch): New functions.
> * config/i386/i386.h (SWITCHABLE_TARGET): Define.
> * config/i386/i386.c: Include target-globals.h.
> (ix86_set_current_function): Instead of doing target_reinit
> unconditionally, use save_target_globals_default_opts and
> restore_target_globals.
> c-family/
> * c-pch.c (c_common_write_pch): Call
> prepare_target_option_nodes_for_pch.
>
> --- gcc/tree-core.h.jj 2014-01-07 08:47:24.000000000 +0100
> +++ gcc/tree-core.h 2014-01-07 16:44:35.591358235 +0100
> @@ -1557,11 +1557,18 @@ struct GTY(()) tree_optimization_option
> struct target_optabs *GTY ((skip)) base_optabs;
> };
>
> +/* Forward declaration, defined in target-globals.h. */
> +
> +struct GTY(()) target_globals;
> +
> /* Target options used by a function. */
>
> struct GTY(()) tree_target_option {
> struct tree_common common;
>
> + /* Target globals for the corresponding target option. */
> + struct target_globals *globals;
> +
> /* The optimization options used by the user. */
> struct cl_target_option opts;
> };
> --- gcc/tree.h.jj 2014-01-03 11:40:33.000000000 +0100
> +++ gcc/tree.h 2014-01-07 21:28:15.038061120 +0100
> @@ -2695,9 +2695,14 @@ extern tree build_optimization_node (str
> #define TREE_TARGET_OPTION(NODE) \
> (&TARGET_OPTION_NODE_CHECK (NODE)->target_option.opts)
>
> +#define TREE_TARGET_GLOBALS(NODE) \
> + (TARGET_OPTION_NODE_CHECK (NODE)->target_option.globals)
> +
> /* Return a tree node that encapsulates the target options in OPTS. */
> extern tree build_target_option_node (struct gcc_options *opts);
>
> +extern void prepare_target_option_nodes_for_pch (void);
> +
> #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007)
>
> inline tree
> --- gcc/target-globals.h.jj 2014-01-03 11:40:46.000000000 +0100
> +++ gcc/target-globals.h 2014-01-07 17:08:51.113880947 +0100
> @@ -37,6 +37,7 @@ extern struct target_builtins *this_targ
> extern struct target_gcse *this_target_gcse;
> extern struct target_bb_reorder *this_target_bb_reorder;
> extern struct target_lower_subreg *this_target_lower_subreg;
> +#endif
>
> struct GTY(()) target_globals {
> struct target_flag_state *GTY((skip)) flag_state;
> @@ -57,6 +58,7 @@ struct GTY(()) target_globals {
> struct target_lower_subreg *GTY((skip)) lower_subreg;
> };
>
> +#if SWITCHABLE_TARGET
> extern struct target_globals default_target_globals;
>
> extern struct target_globals *save_target_globals (void);
> --- gcc/tree.c.jj 2014-01-03 11:40:33.000000000 +0100
> +++ gcc/tree.c 2014-01-07 21:27:35.590268195 +0100
> @@ -11527,6 +11527,28 @@ build_target_option_node (struct gcc_opt
> return t;
> }
>
> +/* Reset TREE_TARGET_GLOBALS cache for TARGET_OPTION_NODE.
> + Called through htab_traverse. */
> +
> +static int
> +prepare_target_option_node_for_pch (void **slot, void *)
> +{
> + tree node = (tree) *slot;
> + if (TREE_CODE (node) == TARGET_OPTION_NODE)
> + TREE_TARGET_GLOBALS (node) = NULL;
> + return 1;
> +}
> +
> +/* Clear TREE_TARGET_GLOBALS of all TARGET_OPTION_NODE trees,
> + so that they aren't saved during PCH writing. */
> +
> +void
> +prepare_target_option_nodes_for_pch (void)
> +{
> + htab_traverse (cl_option_hash_table, prepare_target_option_node_for_pch,
> + NULL);
> +}
> +
> /* Determine the "ultimate origin" of a block. The block may be an inlined
> instance of an inlined instance of a block which is local to an inline
> function, so we have to trace all of the way back through the origin chain
> --- gcc/c-family/c-pch.c.jj 2014-01-03 11:40:29.000000000 +0100
> +++ gcc/c-family/c-pch.c 2014-01-07 21:29:42.985600418 +0100
> @@ -180,6 +180,8 @@ c_common_write_pch (void)
>
> (*debug_hooks->handle_pch) (1);
>
> + prepare_target_option_nodes_for_pch ();
> +
> cpp_write_pch_deps (parse_in, pch_outfile);
>
> gt_pch_save (pch_outfile);
> --- gcc/config/i386/i386.h.jj 2014-01-06 22:37:19.000000000 +0100
> +++ gcc/config/i386/i386.h 2014-01-07 17:21:09.049106675 +0100
> @@ -2510,6 +2510,9 @@ extern void debug_dispatch_window (int);
> #define IX86_HLE_ACQUIRE (1 << 16)
> #define IX86_HLE_RELEASE (1 << 17)
>
> +/* For switching between functions with different target attributes. */
> +#define SWITCHABLE_TARGET 1
> +
> /*
> Local variables:
> version-control: t
> --- gcc/config/i386/i386.c.jj 2014-01-06 22:37:19.000000000 +0100
> +++ gcc/config/i386/i386.c 2014-01-07 17:21:09.049106675 +0100
> @@ -80,6 +80,7 @@ along with GCC; see the file COPYING3.
> #include "tree-pass.h"
> #include "context.h"
> #include "pass_manager.h"
> +#include "target-globals.h"
>
> static rtx legitimize_dllimport_symbol (rtx, bool);
> static rtx legitimize_pe_coff_extern_decl (rtx, bool);
> @@ -4868,16 +4869,25 @@ ix86_set_current_function (tree fndecl)
> {
> cl_target_option_restore (&global_options,
> TREE_TARGET_OPTION (new_tree));
> - target_reinit ();
> + 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)
> {
> - struct cl_target_option *def
> - = TREE_TARGET_OPTION (target_option_current_node);
> -
> - cl_target_option_restore (&global_options, def);
> - target_reinit ();
> + 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 ();
> }
> }
> }
>
>
> Jakub
