From patchwork Mon Oct 17 02:09:04 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lawrence Crowl X-Patchwork-Id: 120085 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 36C94B6F91 for ; Mon, 17 Oct 2011 13:09:40 +1100 (EST) Received: (qmail 21627 invoked by alias); 17 Oct 2011 02:09:35 -0000 Received: (qmail 21590 invoked by uid 22791); 17 Oct 2011 02:09:31 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 Oct 2011 02:09:09 +0000 Received: from hpaq12.eem.corp.google.com (hpaq12.eem.corp.google.com [172.25.149.12]) by smtp-out.google.com with ESMTP id p9H2979w006700; Sun, 16 Oct 2011 19:09:07 -0700 Received: from jade.mtv.corp.google.com (jade.mtv.corp.google.com [172.18.110.116]) by hpaq12.eem.corp.google.com with ESMTP id p9H295DU006774; Sun, 16 Oct 2011 19:09:05 -0700 Received: by jade.mtv.corp.google.com (Postfix, from userid 21482) id A409722266C; Sun, 16 Oct 2011 19:09:04 -0700 (PDT) To: reply@codereview.appspotmail.com, dnovillo@google.com, gcc-patches@gcc.gnu.org Subject: [pph] Refactor Vectors and Chains (issue5263051) Message-Id: <20111017020904.A409722266C@jade.mtv.corp.google.com> Date: Sun, 16 Oct 2011 19:09:04 -0700 (PDT) From: crowl@google.com (Lawrence Crowl) X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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. --- This patch is available for review at http://codereview.appspot.com/5263051 Index: gcc/testsuite/ChangeLog.pph 2011-10-16 Lawrence Crowl * 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 * 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) 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); }