Re: [pph] Add streamer hook for preloading common nodes (issue4478043)
On Thu, May 5, 2011 at 07:07, Richard Guenther wrote: > For LTO we have type-merging for that, and we'd continue to pre-load > the type merger with the (LTO frontend specific) common tree nodes. OK. For LTO it may make sense to eventually make this hook a nop, then. > I suppose you are not doing any merging at all? If so pre-loading those > nodes makes indeed sense (given you have a way to reject PPH images > when flags such as -f[un]signed-char differ ...). There will be some amount of merging, but I'm anticipating using the same merging scheme used by the parser. As far as the parser is concerned, pph images are not much different than a regular header file. What changes is the way those declarations get loaded in memory. Diego.
Re: [pph] Add streamer hook for preloading common nodes (issue4478043)
On Thu, May 5, 2011 at 1:03 PM, Diego Novillo wrote: > On Thu, May 5, 2011 at 05:06, Richard Guenther > wrote: > >> 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). > > Sure, though that's not an issue for pph. PPH images are generated > and consumed by one front end. > > Pre-loading common nodes in C++ gives us: > > 1- Smaller pph images > 2- Ability to do pointer comparison against common nodes. > > #1 saves us from saving a few hundred nodes in each pph image. But #2 > is a stronger requirement. How do you think we could do #2 without > pre-loading? For LTO we have type-merging for that, and we'd continue to pre-load the type merger with the (LTO frontend specific) common tree nodes. I suppose you are not doing any merging at all? If so pre-loading those nodes makes indeed sense (given you have a way to reject PPH images when flags such as -f[un]signed-char differ ...). Richard. > > Diego. >
Re: [pph] Add streamer hook for preloading common nodes (issue4478043)
On Thu, May 5, 2011 at 05:06, Richard Guenther wrote: > 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). Sure, though that's not an issue for pph. PPH images are generated and consumed by one front end. Pre-loading common nodes in C++ gives us: 1- Smaller pph images 2- Ability to do pointer comparison against common nodes. #1 saves us from saving a few hundred nodes in each pph image. But #2 is a stronger requirement. How do you think we could do #2 without pre-loading? Diego.
Re: [pph] Add streamer hook for preloading common nodes (issue4478043)
On Wed, May 4, 2011 at 10:26 PM, Diego Novillo 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_commo
[pph] Add streamer hook for preloading common nodes (issue4478043)
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.
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 (e
