On Wed, May 4, 2011 at 10:26 PM, Diego Novillo <dnovi...@google.com> wrote:
>
> This patch adds a new streamer hook to populate the streamer cache
> with common nodes.
>
> The nodes we populate for GIMPLE in lto_get_common_nodes is not
> sufficient for C++, unsurprisingly.
>
> The patch fixes these regressions in pph.exp:
>
> FAIL: g++.dg/pph/p1stdlib.cc -fpph-map=pph.map -I. (test for excess errors)
> FAIL: g++.dg/pph/p1stdlib.cc , PPH assembly missing
>
> There is a second part to this patch to handle INTEGER_CSTs as regular
> trees (so they can be cached). This would allow us to handle special
> constants in the C++ FE like void_zero_node, but that's giving me some
> trouble with LTO tests.
>
> Tested on x86_64. Committed to the branch.
I think we should move away from pre-loading the streamer cache, that
has caused enough trouble when the common nodes are originating from
different Frontends and when compiling units with different flags which
happen to change those nodes (think of the hoops we jump through
to support that for -f[un]signed-char).
Richard.
>
> Diego.
>
> ChangeLog.pph
>
> * Makefile.in (cgraphunit.o): Add dependency on LTO_STREAMER_H.
> * cgraphunit.c: Include lto-streamer.h
> (cgraph_finalize_compilation_unit): Call gimple_streamer_hooks_init
> if LTO is enabled.
> * lto-streamer-out.c (lto_output): Move call to
> gimple_streamer_hooks_init to cgraph_finalize_compilation_unit.
> * lto-streamer.c (lto_get_common_nodes): Remove if0 hack.
> (lto_streamer_cache_create): Call streamer_hooks.get_common_nodes
> instead of lto_get_common_nodes.
> (gimple_streamer_hooks_init): Set h->get_common_nodes to
> lto_get_common_nodes.
> * lto-streamer.h (struct lto_streamer_hooks): Add field
> get_common_nodes.
>
> cp/ChangeLog.pph
>
> * pph-streamer.c (pph_get_common_nodes): New.
> (pph_stream_hooks_init): Set it in h->get_common_nodes.
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 0af93ba..f96e059 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3001,7 +3001,7 @@ cgraphunit.o : cgraphunit.c $(CONFIG_H) $(SYSTEM_H)
> coretypes.h $(TM_H) \
> $(TREE_FLOW_H) $(TREE_PASS_H) debug.h $(DIAGNOSTIC_H) \
> $(FIBHEAP_H) output.h $(PARAMS_H) $(RTL_H) $(TIMEVAR_H) $(IPA_PROP_H) \
> gt-cgraphunit.h tree-iterator.h $(COVERAGE_H) $(TREE_DUMP_H) \
> - tree-pretty-print.h gimple-pretty-print.h
> + tree-pretty-print.h gimple-pretty-print.h $(LTO_STREAMER_H)
> cgraphbuild.o : cgraphbuild.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
> $(TREE_H) langhooks.h $(CGRAPH_H) intl.h pointer-set.h $(GIMPLE_H) \
> $(TREE_FLOW_H) $(TREE_PASS_H) $(IPA_UTILS_H) $(EXCEPT_H) \
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 3dbfc2b..0d1ec89 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -138,6 +138,7 @@ along with GCC; see the file COPYING3. If not see
> #include "output.h"
> #include "coverage.h"
> #include "plugin.h"
> +#include "lto-streamer.h"
>
> static void cgraph_expand_all_functions (void);
> static void cgraph_mark_functions_to_output (void);
> @@ -1062,6 +1063,10 @@ cgraph_finalize_compilation_unit (void)
> {
> timevar_push (TV_CGRAPH);
>
> + /* If LTO is enabled, initialize the streamer hooks needed by GIMPLE. */
> + if (flag_lto)
> + gimple_streamer_hooks_init ();
> +
> /* If we're here there's no current function anymore. Some frontends
> are lazy in clearing these. */
> current_function_decl = NULL;
> diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
> index 5f2112e..c363c06 100644
> --- a/gcc/cp/pph-streamer.c
> +++ b/gcc/cp/pph-streamer.c
> @@ -55,6 +55,36 @@ pph_indexable_with_decls_p (tree t)
> }
>
>
> +/* Generate a vector of common nodes that should always be streamed as
> + indexes into the streamer cache. These nodes are always built by
> + the front end, so there is no need to emit them. */
> +
> +static VEC(tree,heap) *
> +pph_get_common_nodes (void)
> +{
> + unsigned i;
> + VEC(tree,heap) *common_nodes = NULL;
> +
> + for (i = itk_char; i < itk_none; i++)
> + VEC_safe_push (tree, heap, common_nodes, integer_types[i]);
> +
> + for (i = 0; i < TYPE_KIND_LAST; i++)
> + VEC_safe_push (tree, heap, common_nodes, sizetype_tab[i]);
> +
> + /* global_trees[] can have NULL entries in it. Skip them. */
> + for (i = 0; i < TI_MAX; i++)
> + if (global_trees[i])
> + VEC_safe_push (tree, heap, common_nodes, global_trees[i]);
> +
> + /* c_global_trees[] can have NULL entries in it. Skip them. */
> + for (i = 0; i < CTI_MAX; i++)
> + if (c_global_trees[i])
> + VEC_safe_push (tree, heap, common_nodes, c_global_trees[i]);
> +
> + return common_nodes;
> +}
> +
> +
> /* Initialize all the streamer hooks used for streaming ASTs. */
>
> static void
> @@ -62,6 +92,9 @@ pph_stream_hooks_init (void)
> {
> lto_streamer_hooks *h = streamer_hooks_init ();
> h->name = "C++ AST";
> + h->get_common_nodes = pph_get_common_nodes;
> h->is_streamable = pph_is_streamable;
> h->write_tree = pph_stream_write_tree;
> h->read_tree = pph_stream_read_tree;
> diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
> index c26de5d..b578419 100644
> --- a/gcc/lto-streamer-out.c
> +++ b/gcc/lto-streamer-out.c
> @@ -2198,7 +2198,6 @@ lto_output (cgraph_node_set set, varpool_node_set vset)
> int i, n_nodes;
> lto_cgraph_encoder_t encoder = lto_get_out_decl_state
> ()->cgraph_node_encoder;
>
> - gimple_streamer_hooks_init ();
> lto_writer_init ();
>
> n_nodes = lto_cgraph_encoder_size (encoder);
> diff --git a/gcc/lto-streamer.c b/gcc/lto-streamer.c
> index 04a9f7a..d6b78c8 100644
> --- a/gcc/lto-streamer.c
> +++ b/gcc/lto-streamer.c
> @@ -550,10 +550,6 @@ lto_get_common_nodes (void)
> else
> main_identifier_node = get_identifier ("main");
>
> - /* FIXME pph. These assertions are never met while in the front end.
> - There should be a way of checking this only when we are in LTO
> - mode. */
> -#if 0
> gcc_assert (ptrdiff_type_node == integer_type_node);
>
> /* FIXME lto. In the C++ front-end, fileptr_type_node is defined as a
> @@ -564,7 +560,6 @@ lto_get_common_nodes (void)
> These should be assured in pass_ipa_free_lang_data. */
> gcc_assert (fileptr_type_node == ptr_type_node);
> gcc_assert (TYPE_MAIN_VARIANT (fileptr_type_node) == ptr_type_node);
> -#endif
>
> seen_nodes = pointer_set_create ();
>
> @@ -631,7 +626,7 @@ lto_streamer_cache_create (void)
> /* Load all the well-known tree nodes that are always created by
> the compiler on startup. This prevents writing them out
> unnecessarily. */
> - common_nodes = lto_get_common_nodes ();
> + common_nodes = streamer_hooks ()->get_common_nodes ();
>
> FOR_EACH_VEC_ELT (tree, common_nodes, i, node)
> preload_common_node (cache, node);
> @@ -820,6 +815,7 @@ gimple_streamer_hooks_init (void)
> h->name = "gimple";
> h->reader_init = gimple_streamer_reader_init;
> h->writer_init = NULL;
> + h->get_common_nodes = lto_get_common_nodes;
> h->is_streamable = lto_is_streamable;
> h->write_tree = gimple_streamer_write_tree;
> h->read_tree = gimple_streamer_read_tree;
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index b6f4b79..8be17da 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -55,6 +55,16 @@ typedef struct lto_streamer_hooks {
> /* A string identifying this streamer. */
> const char *name;
>
> + /* Called by lto_streamer_cache_create to instantiate a cache of
> + well-known nodes. These are tree nodes that are always
> + instantiated by the compiler on startup. Additionally, these
> + nodes need to be shared. This function produces a vector of
> + these well known trees that are then added to the streamer cache.
> + This way, the writer will only write out a reference to the tree
> + and the reader will instantiate the tree out of this
> + pre-populated cache. */
> + VEC(tree,heap) *(*get_common_nodes) (void);
> +
> /* Called by lto_reader_init after it does basic initialization. */
> void (*reader_init) (void);
>
>
> --
> This patch is available for review at http://codereview.appspot.com/4478043
>
