* [pph] Refactor Vectors and Chains (issue5263051)
@ 2011-10-17 6:44 Lawrence Crowl
2011-10-17 13:04 ` Diego Novillo
0 siblings, 1 reply; 2+ messages in thread
From: Lawrence Crowl @ 2011-10-17 6:44 UTC (permalink / raw)
To: reply, dnovillo, gcc-patches
Factor the vector and chain output routines to remove boolean control
parameters. The functions pph_out_tree_vec_1 and pph_out_chain_1 split
their conditional parts of their implementation into their use cases,
calling each other as needed.
pph_out_tree_vec - nothing special
pph_out_tree_vec_unchain - unchaining
pph_out_mergeable_tree_vec - merging, unchaining, reversing
pph_out_tree_vec_filtered - filtering
pph_out_chain - nothing special
pph_out_chain_filtered - filtering
pph_out_mergeable_chain_filtered - merging, unchaining, reversing, filtering
This change fixes an ordering bug, but now causes an ICE to surface,
which will be addressed later.
Tested on x64.
Index: gcc/testsuite/ChangeLog.pph
2011-10-16 Lawrence Crowl <crowl@google.com>
* g++.dg/pph/x4tmplfuncinln.cc: Change failure to ICE.
* g++.dg/pph/x4tmplfuncninl.cc: Likewise.
* g++.dg/pph/z4tmplfuncinln.cc: Likewise.
* g++.dg/pph/z4tmplfuncninl.cc: Likewise.
Index: gcc/cp/ChangeLog.pph
2011-10-16 Lawrence Crowl <crowl@google.com>
* pph-streamer.h (pph_dump_chain): New.
* pph-streamer.c (pph_trace_tree): Filter expressions from trace.
Print tree code.
* pph.c (pph_dump_tree_name): Add print newline.
(pph_dump_namespace): Remove print space.
(pph_dump_binding): Factor out dumping a chain.
(pph_dump_chain): New.
* pph-streamer-in.c (pph_in_mergeable_binding_level): Use
pph_dump_chain.
(pph_read_file_1): Change location of namespace dump.
* pph-streamer-out.c (pph_tree_matches): Remove redundant test.
(pph_out_tree_vec_1): Removed.
(pph_out_tree_vec): Takes core implementation from pph_out_tree_vec_1.
(pph_out_tree_vec_unchain): New. Takes core implementation from
pph_out_tree_vec_1.
(pph_out_mergeable_tree_vec): Likewise.
(pph_out_tree_vec_filtered): Likewise.
(chain2vec_filter): Prevent null filter.
(chain2vec): New. Handles case above.
(pph_out_chain_1): Removed.
(pph_out_chain): Takes core implementation from pph_out_chain_1.
(pph_out_chain_filtered): New. Takes core implementation from
pph_out_chain_1.
(pph_out_mergeable_chain_filtered): Likewise.
Index: gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (working copy)
@@ -1,5 +1,6 @@
-// pph asm xdiff 37887
-// xfail BOGUS DIFF LABEL
+// { dg-xfail-if "ICE" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "a0tmplfuncninl_g.h:12:23: internal compiler error: in instantiate_decl" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "Template list problems" }
#include "x0tmplfuncninl1.h"
#include "x0tmplfuncninl2.h"
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (working copy)
@@ -1,5 +1,6 @@
-// pph asm xdiff 05125
-// xfail BOGUS DUPFUN
+// { dg-xfail-if "ICE" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "a0tmplfuncninl_g.h:12:23: internal compiler error: in instantiate_decl" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "Template list problems" }
#include "x0tmplfuncninl3.h"
#include "x0tmplfuncninl4.h"
Index: gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc (working copy)
@@ -1,6 +1,6 @@
-// pph asm xdiff 16845
-// xfail BOGUS DUPFUN
-// double function1<double>(double) is duplicated
+// { dg-xfail-if "ICE" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "a0tmplfuncinln_g.h:17:23: internal compiler error: in instantiate_decl" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "Template list problems" }
#include "x0tmplfuncinln1.h"
#include "x0tmplfuncinln2.h"
Index: gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc
===================================================================
--- gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc (revision 180048)
+++ gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc (working copy)
@@ -1,5 +1,6 @@
-// pph asm xdiff 65129
-// xfail BOGUS DUPFUNC
+// { dg-xfail-if "ICE" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "a0tmplfuncinln_g.h:17:23: internal compiler error: in instantiate_decl" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "Template list problems" }
#include "x0tmplfuncinln3.h"
#include "x0tmplfuncinln4.h"
Index: gcc/cp/pph.c
===================================================================
--- gcc/cp/pph.c (revision 180048)
+++ gcc/cp/pph.c (working copy)
@@ -101,7 +101,10 @@ pph_dump_tree_name (FILE *file, tree t,
else if (EXPR_P (t))
fprintf (file, "%s\n", expr_as_string (t, flags));
else
- print_generic_expr (file, t, flags);
+ {
+ print_generic_expr (file, t, flags);
+ fprintf (file, "\n");
+ }
#endif
}
@@ -113,7 +116,7 @@ pph_dump_namespace (FILE *file, tree ns)
{
fprintf (file, "namespace ");
pph_dump_tree_name (file, ns, 0);
- fprintf (file, " {\n");
+ fprintf (file, "{\n");
pph_dump_binding (file, NAMESPACE_LEVEL (ns));
fprintf (file, "}\n");
}
@@ -124,19 +127,28 @@ pph_dump_namespace (FILE *file, tree ns)
void
pph_dump_binding (FILE *file, cp_binding_level *level)
{
- tree t, chain;
-
- for (t = level->names; t; t = chain)
+ tree t, next;
+ pph_dump_chain (file, level->names);
+ for (t = level->namespaces; t; t = next)
{
- chain = DECL_CHAIN (t);
+ next = DECL_CHAIN (t);
if (!DECL_IS_BUILTIN (t))
- pph_dump_tree_name (file, t, 0);
+ pph_dump_namespace (file, t);
}
- for (t = level->namespaces; t; t = chain)
+}
+
+
+/* Dump a CHAIN for PPH. */
+
+void
+pph_dump_chain (FILE *file, tree chain)
+{
+ tree t, next;
+ for (t = chain; t; t = next)
{
- chain = DECL_CHAIN (t);
+ next = DECL_CHAIN (t);
if (!DECL_IS_BUILTIN (t))
- pph_dump_namespace (file, t);
+ pph_dump_tree_name (file, t, 0);
}
}
Index: gcc/cp/pph-streamer-in.c
===================================================================
--- gcc/cp/pph-streamer-in.c (revision 180048)
+++ gcc/cp/pph-streamer-in.c (working copy)
@@ -1086,7 +1086,8 @@ pph_in_mergeable_binding_level (cp_bindi
if (flag_pph_debug >= 3)
{
fprintf (pph_logfile, "PPH: Merging into this chain:\n");
- debug_tree_chain (existing->names);
+ pph_dump_chain (pph_logfile, existing->names);
+ fprintf (pph_logfile, "\n");
}
pph_in_mergeable_chain (stream, &existing->names);
pph_in_mergeable_chain (stream, &existing->namespaces);
@@ -2377,9 +2378,6 @@ pph_read_file_1 (pph_stream *stream)
/* Read the bindings from STREAM and merge them with the current bindings. */
pph_in_mergeable_binding_level (scope_chain->bindings, stream);
- if (flag_pph_dump_tree)
- pph_dump_namespace (pph_logfile, global_namespace);
-
/* Read and merge the other global state collected during parsing of
the original header. */
file_keyed_classes = pph_in_tree (stream);
@@ -2401,6 +2399,9 @@ pph_read_file_1 (pph_stream *stream)
/* Mark this file as read. If other images need to access its contents,
we will not need to actually read it again. */
pph_mark_stream_read (stream);
+
+ if (flag_pph_dump_tree)
+ pph_dump_namespace (pph_logfile, global_namespace);
}
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c (revision 180048)
+++ gcc/cp/pt.c (working copy)
@@ -20301,9 +20301,7 @@ pph_dump_spec_entry_slot (void **slot, v
struct spec_entry *entry = (struct spec_entry *) *slot;
fprintf (stream, "spec_entry.tmpl: " );
pph_dump_tree_name (stream, entry->tmpl, 0);
- fprintf (stream, "spec_entry.args: " );
- pph_dump_tree_name (stream, entry->args, 0);
- fprintf (stream, "\nspec_entry.spec: " );
+ fprintf (stream, "spec_entry.spec: " );
pph_dump_tree_name (stream, entry->spec, 0);
return 1;
}
Index: gcc/cp/pph-streamer.c
===================================================================
--- gcc/cp/pph-streamer.c (revision 180048)
+++ gcc/cp/pph-streamer.c (working copy)
@@ -345,12 +345,14 @@ pph_trace_tree (tree t, bool mergeable)
emit = true;
else if ((mergeable || is_decl) && flag_pph_tracer >= 3)
emit = true;
- else if (flag_pph_tracer >= 4)
+ else if (!EXPR_P (t) && flag_pph_tracer >= 4)
emit = true;
if (emit)
{
+ enum tree_code code = TREE_CODE (t);
fprintf (pph_logfile, "PPH: %c%c ", merging, userdef);
+ fprintf (pph_logfile, "%-19s ", pph_tree_code_text (code));
pph_dump_tree_name (pph_logfile, t, 0);
}
}
Index: gcc/cp/pph-streamer.h
===================================================================
--- gcc/cp/pph-streamer.h (revision 180048)
+++ gcc/cp/pph-streamer.h (working copy)
@@ -220,6 +220,7 @@ struct pph_stream {
/* In pph.c */
extern const char *pph_tree_code_text (enum tree_code code);
extern void pph_dump_min_decl (FILE *file, tree decl);
+extern void pph_dump_chain (FILE *, tree chain);
extern void pph_dump_binding (FILE *, cp_binding_level *level);
extern void pph_dump_namespace (FILE *, tree ns);
Index: gcc/cp/pph-streamer-out.c
===================================================================
--- gcc/cp/pph-streamer-out.c (revision 180048)
+++ gcc/cp/pph-streamer-out.c (working copy)
@@ -753,15 +753,17 @@ pph_out_token_cache (pph_stream *f, cp_t
/******************************************************************* vectors */
+/* Note that we use the same format used by streamer_write_chain.
+ This is to support pph_out_chain_filtered, which writes the
+ filtered chain as a VEC. Since the reader always reads chains
+ using streamer_read_chain, we have to write VECs in exactly the
+ same way as tree chains. */
/* Return true if T matches FILTER for STREAM. */
static inline bool
pph_tree_matches (pph_stream *stream, tree t, unsigned filter)
{
- if (filter == PPHF_NONE)
- return true;
-
if ((filter & PPHF_NO_BUILTINS)
&& DECL_P (t)
&& DECL_IS_BUILTIN (t))
@@ -804,73 +806,56 @@ vec2vec_filter (pph_stream *stream, VEC(
}
-/* Write all the trees in VEC V to STREAM. REVERSE is true if V should
- be written in reverse. MERGEABLE is true if the tree nodes in V
- are mergeable trees (see pph_out_tree_1). If FILTER is set,
- only emit the elements in V that match it. If UNCHAIN is true,
- clear TREE_CHAIN on every element written out (this is to support
- writing chains, as they are supposed to be re-chained by the
- reader). */
+/* Write all the trees in VEC V to STREAM. */
static void
-pph_out_tree_vec_1 (pph_stream *stream, VEC(tree,gc) *v, unsigned filter,
- bool mergeable, bool reverse, bool unchain)
+pph_out_tree_vec (pph_stream *stream, VEC(tree,gc) *v)
{
unsigned i;
tree t;
- VEC(tree,heap) *to_write;
-
- if (filter != PPHF_NONE)
- to_write = vec2vec_filter (stream, v, filter);
- else
- to_write = (VEC(tree,heap) *) v;
+ pph_out_hwi (stream, VEC_length (tree, v));
+ FOR_EACH_VEC_ELT (tree, v, i, t)
+ pph_out_tree (stream, t);
+}
- /* Note that we use the same format used by streamer_write_chain.
- This is to support pph_out_chain_filtered, which writes the
- filtered chain as a VEC. Since the reader always reads chains
- using streamer_read_chain, we have to write VECs in exactly the
- same way as tree chains. */
- pph_out_hwi (stream, VEC_length (tree, to_write));
- if (!reverse)
- FOR_EACH_VEC_ELT (tree, to_write, i, t)
- {
- tree prev = NULL;
- if (unchain)
- {
- prev = TREE_CHAIN (t);
- TREE_CHAIN (t) = NULL;
- }
- pph_out_tree_1 (stream, t, mergeable);
- if (unchain)
- TREE_CHAIN (t) = prev;
- }
- else
- FOR_EACH_VEC_ELT_REVERSE (tree, to_write, i, t)
- {
- tree prev = NULL;
- if (unchain)
- {
- prev = TREE_CHAIN (t);
- TREE_CHAIN (t) = NULL;
- }
- pph_out_tree_1 (stream, t, mergeable);
- if (unchain)
- TREE_CHAIN (t) = prev;
- }
+/* Write all the trees in VEC V to STREAM.
+ Clear TREE_CHAIN on every element written out (this is to support
+ writing chains, as they are supposed to be re-chained by the reader). */
- /* If we did not have to filter, TO_WRITE == V. Do not free it! */
- if (filter != PPHF_NONE)
- VEC_free (tree, heap, to_write);
+static void
+pph_out_tree_vec_unchain (pph_stream *stream, VEC(tree,gc) *v)
+{
+ unsigned i;
+ tree t;
+ pph_out_hwi (stream, VEC_length (tree, v));
+ FOR_EACH_VEC_ELT (tree, v, i, t)
+ {
+ tree prev = TREE_CHAIN (t);
+ TREE_CHAIN (t) = NULL;
+ pph_out_tree (stream, t);
+ TREE_CHAIN (t) = prev;
+ }
}
-/* Write all the trees in VEC V to STREAM. */
+/* Write all the mergeable trees in VEC V to STREAM.
+ The trees must go out in declaration order, i.e. reversed.
+ Unchain each before writing and rechain after writing. */
static void
-pph_out_tree_vec (pph_stream *stream, VEC(tree,gc) *v)
+pph_out_mergeable_tree_vec (pph_stream *stream, VEC(tree,gc) *v)
{
- pph_out_tree_vec_1 (stream, v, PPHF_NONE, false, false, false);
+ unsigned i;
+ tree t;
+ pph_out_hwi (stream, VEC_length (tree, v));
+ FOR_EACH_VEC_ELT_REVERSE (tree, v, i, t)
+ {
+ tree prev = TREE_CHAIN (t);
+ TREE_CHAIN (t) = NULL;
+ pph_out_tree_1 (stream, t, /*mergeable=*/true);
+ TREE_CHAIN (t) = prev;
+ }
}
@@ -879,7 +864,14 @@ pph_out_tree_vec (pph_stream *stream, VE
static void
pph_out_tree_vec_filtered (pph_stream *stream, VEC(tree,gc) *v, unsigned filter)
{
- pph_out_tree_vec_1 (stream, v, filter, false, false, false);
+ if (filter == PPHF_NONE)
+ pph_out_tree_vec (stream, v);
+ else
+ {
+ VEC(tree,heap) *w = vec2vec_filter (stream, v, filter);
+ pph_out_tree_vec (stream, (VEC(tree,gc) *)w);
+ VEC_free (tree, heap, w);
+ }
}
@@ -929,6 +921,11 @@ chain2vec_filter (pph_stream *stream, tr
tree t;
VEC(tree,heap) *v = NULL;
+ /* Do not accept the nil filter. The caller is responsible for
+ freeing the returned vector and they may inadvertently free
+ a vector they assumed to be allocated by this function. */
+ gcc_assert (filter != PPHF_NONE);
+
for (t = chain; t; t = TREE_CHAIN (t))
if (pph_tree_matches (stream, t, filter))
VEC_safe_push (tree, heap, v, t);
@@ -937,30 +934,18 @@ chain2vec_filter (pph_stream *stream, tr
}
-/* Write a chain of trees to STREAM starting with FIRST (if REVERSE is
- false) or the last element reachable from FIRST (if REVERSE is
- true). If FILTER is given, use it to decide what nodes should be
- emitted. MERGEABLE is as in pph_out_tree_1. */
+/* Convert a CHAIN to a VEC by copying the nodes. */
-static void
-pph_out_chain_1 (pph_stream *stream, tree first, unsigned filter,
- bool mergeable, bool reverse)
+static VEC(tree,heap) *
+chain2vec (tree chain)
{
- VEC(tree,heap) *vec = NULL;
+ tree t;
+ VEC(tree,heap) *v = NULL;
- /* Unmodified chain writes go directly to the regular tree streamer. */
- if (filter == PPHF_NONE && !mergeable && !reverse)
- {
- streamer_write_chain (stream->encoder.w.ob, first, false);
- return;
- }
+ for (t = chain; t; t = TREE_CHAIN (t))
+ VEC_safe_push (tree, heap, v, t);
- /* For everything else, convert the chain into a VEC and write it
- out. Since we have already applied FILTER, there is no need to
- apply it again. */
- vec = chain2vec_filter (stream, first, filter);
- pph_out_tree_vec_1 (stream, (VEC(tree,gc) *)vec, reverse, mergeable,
- PPHF_NONE, true);
+ return v;
}
@@ -969,7 +954,7 @@ pph_out_chain_1 (pph_stream *stream, tre
static void
pph_out_chain (pph_stream *stream, tree first)
{
- pph_out_chain_1 (stream, first, PPHF_NONE, false, false);
+ streamer_write_chain (stream->encoder.w.ob, first, false);
}
@@ -979,7 +964,14 @@ pph_out_chain (pph_stream *stream, tree
static void
pph_out_chain_filtered (pph_stream *stream, tree first, unsigned filter)
{
- pph_out_chain_1 (stream, first, filter, false, false);
+ if (filter == PPHF_NONE)
+ pph_out_chain (stream, first);
+ else
+ {
+ VEC(tree,heap) *w = chain2vec_filter (stream, first, filter);
+ pph_out_tree_vec_unchain (stream, (VEC(tree,gc) *)w);
+ VEC_free (tree, heap, w);
+ }
}
@@ -991,7 +983,13 @@ static void
pph_out_mergeable_chain_filtered (pph_stream *stream, tree chain,
unsigned filter)
{
- pph_out_chain_1 (stream, chain, filter, true, true);
+ VEC(tree,heap) *w;
+ if (filter == PPHF_NONE)
+ w = chain2vec (chain);
+ else
+ w = chain2vec_filter (stream, chain, filter);
+ pph_out_mergeable_tree_vec (stream, (VEC(tree,gc) *)w);
+ VEC_free (tree, heap, w);
}
@@ -2247,8 +2245,6 @@ pph_write_file (pph_stream *stream)
/* Emit the bindings for the global namespace. */
pph_out_global_binding (stream);
- if (flag_pph_dump_tree)
- pph_dump_namespace (pph_logfile, global_namespace);
/* Emit other global state kept by the parser. FIXME pph, these
globals should be fields in struct cp_parser. */
@@ -2262,6 +2258,9 @@ pph_write_file (pph_stream *stream)
/* Emit the symbol table. */
pph_out_symtab (stream);
+
+ if (flag_pph_dump_tree)
+ pph_dump_namespace (pph_logfile, global_namespace);
}
--
This patch is available for review at http://codereview.appspot.com/5263051
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [pph] Refactor Vectors and Chains (issue5263051)
2011-10-17 6:44 [pph] Refactor Vectors and Chains (issue5263051) Lawrence Crowl
@ 2011-10-17 13:04 ` Diego Novillo
0 siblings, 0 replies; 2+ messages in thread
From: Diego Novillo @ 2011-10-17 13:04 UTC (permalink / raw)
To: Lawrence Crowl; +Cc: reply, gcc-patches
On Sun, Oct 16, 2011 at 22:09, Lawrence Crowl <crowl@google.com> wrote:
> Factor the vector and chain output routines to remove boolean control
> parameters. The functions pph_out_tree_vec_1 and pph_out_chain_1 split
> their conditional parts of their implementation into their use cases,
> calling each other as needed.
>
> pph_out_tree_vec - nothing special
> pph_out_tree_vec_unchain - unchaining
> pph_out_mergeable_tree_vec - merging, unchaining, reversing
> pph_out_tree_vec_filtered - filtering
> pph_out_chain - nothing special
> pph_out_chain_filtered - filtering
> pph_out_mergeable_chain_filtered - merging, unchaining, reversing, filtering
But, you are duplicating code that the previous patch had explicitly
commonized. Why? It's easier to keep the core streaming logic in one
function, to have a single point of debugging when dealing with sync
problems.
> This change fixes an ordering bug, but now causes an ICE to surface,
> which will be addressed later.
What ordering bug?
Diego.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-10-17 12:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-17 6:44 [pph] Refactor Vectors and Chains (issue5263051) Lawrence Crowl
2011-10-17 13:04 ` Diego Novillo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).