* [pph] Fix binding_level's names_size streaming (issue4634071)
@ 2011-06-23 0:17 Gabriel Charette
2011-06-23 2:20 ` Gabriel Charette
0 siblings, 1 reply; 9+ messages in thread
From: Gabriel Charette @ 2011-06-23 0:17 UTC (permalink / raw)
To: reply, gcc-patches
See previous message. This patch only adds the fix to the dg-bogus comment for the failing pph test.
2011-06-22 Gabriel Charette <gchare@google.com>
* gcc/cp/name-lookup.h (cp_binding_level): Removed unused
member names_size. Update all users.
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 54977ce..297c57e 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -544,7 +544,6 @@ add_decl_to_level (tree decl, cxx_scope *b)
necessary. */
TREE_CHAIN (decl) = b->names;
b->names = decl;
- b->names_size++;
/* If appropriate, add decl to separate list of statics. We
include extern variables because they might turn out to be
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 009b5d9..5f266eb 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -191,9 +191,6 @@ struct GTY(()) cp_binding_level {
are wrapped in TREE_LISTs; the TREE_VALUE is the OVERLOAD. */
tree names;
- /* Count of elements in names chain. */
- size_t names_size;
-
/* A chain of NAMESPACE_DECL nodes. */
tree namespaces;
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 0e8c6bf..a535cab 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -442,7 +442,6 @@ pph_in_binding_level (pph_stream *stream)
ALLOC_AND_REGISTER (stream, ix, bl, ggc_alloc_cleared_cp_binding_level ());
bl->names = pph_in_chain (stream);
- bl->names_size = pph_in_uint (stream);
bl->namespaces = pph_in_chain (stream);
bl->static_decls = pph_in_tree_vec (stream);
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index f219cef..7a6c516 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -448,7 +448,6 @@ pph_out_binding_level (pph_stream *stream, struct cp_binding_level *bl,
return;
pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS);
- pph_out_uint (stream, bl->names_size);
pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS);
pph_out_tree_vec (stream, bl->static_decls, ref_p);
diff --git a/gcc/testsuite/g++.dg/pph/x1template.cc b/gcc/testsuite/g++.dg/pph/x1template.cc
index 5b1e980..cecefd7 100644
--- a/gcc/testsuite/g++.dg/pph/x1template.cc
+++ b/gcc/testsuite/g++.dg/pph/x1template.cc
@@ -1,5 +1,5 @@
// { dg-xfail-if "ICE" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "x1template.h:18:13: internal compiler error: in resume_scope, at cp/name-lookup.c:1569" "" { xfail *-*-* } 0 }
+// { dg-bogus "x1template.h:18:13: internal compiler error: in resume_scope, at cp/name-lookup.c:1568" "" { xfail *-*-* } 0 }
// { dg-prune-output "In file included from " }
#include "x1template.h"
--
This patch is available for review at http://codereview.appspot.com/4634071
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pph] Fix binding_level's names_size streaming (issue4634071)
2011-06-23 0:17 [pph] Fix binding_level's names_size streaming (issue4634071) Gabriel Charette
@ 2011-06-23 2:20 ` Gabriel Charette
2011-06-23 4:55 ` Gabriel Dos Reis
0 siblings, 1 reply; 9+ messages in thread
From: Gabriel Charette @ 2011-06-23 2:20 UTC (permalink / raw)
To: reply, gcc-patches, Diego Novillo, Lawrence Crowl
And it looks like this wasn't sent to anyone directly...
Adding back dnovillo and crowl (Diego I don't think Jason was ever
added to the original message...?)
Gab
On Wed, Jun 22, 2011 at 5:02 PM, Gabriel Charette <gchare@google.com> wrote:
>
> See previous message. This patch only adds the fix to the dg-bogus comment for the failing pph test.
>
> 2011-06-22 Gabriel Charette <gchare@google.com>
>
> * gcc/cp/name-lookup.h (cp_binding_level): Removed unused
> member names_size. Update all users.
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 54977ce..297c57e 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -544,7 +544,6 @@ add_decl_to_level (tree decl, cxx_scope *b)
> necessary. */
> TREE_CHAIN (decl) = b->names;
> b->names = decl;
> - b->names_size++;
>
> /* If appropriate, add decl to separate list of statics. We
> include extern variables because they might turn out to be
> diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
> index 009b5d9..5f266eb 100644
> --- a/gcc/cp/name-lookup.h
> +++ b/gcc/cp/name-lookup.h
> @@ -191,9 +191,6 @@ struct GTY(()) cp_binding_level {
> are wrapped in TREE_LISTs; the TREE_VALUE is the OVERLOAD. */
> tree names;
>
> - /* Count of elements in names chain. */
> - size_t names_size;
> -
> /* A chain of NAMESPACE_DECL nodes. */
> tree namespaces;
>
> diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
> index 0e8c6bf..a535cab 100644
> --- a/gcc/cp/pph-streamer-in.c
> +++ b/gcc/cp/pph-streamer-in.c
> @@ -442,7 +442,6 @@ pph_in_binding_level (pph_stream *stream)
> ALLOC_AND_REGISTER (stream, ix, bl, ggc_alloc_cleared_cp_binding_level ());
>
> bl->names = pph_in_chain (stream);
> - bl->names_size = pph_in_uint (stream);
> bl->namespaces = pph_in_chain (stream);
>
> bl->static_decls = pph_in_tree_vec (stream);
> diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
> index f219cef..7a6c516 100644
> --- a/gcc/cp/pph-streamer-out.c
> +++ b/gcc/cp/pph-streamer-out.c
> @@ -448,7 +448,6 @@ pph_out_binding_level (pph_stream *stream, struct cp_binding_level *bl,
> return;
>
> pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS);
> - pph_out_uint (stream, bl->names_size);
> pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS);
>
> pph_out_tree_vec (stream, bl->static_decls, ref_p);
> diff --git a/gcc/testsuite/g++.dg/pph/x1template.cc b/gcc/testsuite/g++.dg/pph/x1template.cc
> index 5b1e980..cecefd7 100644
> --- a/gcc/testsuite/g++.dg/pph/x1template.cc
> +++ b/gcc/testsuite/g++.dg/pph/x1template.cc
> @@ -1,5 +1,5 @@
> // { dg-xfail-if "ICE" { "*-*-*" } { "-fpph-map=pph.map" } }
> -// { dg-bogus "x1template.h:18:13: internal compiler error: in resume_scope, at cp/name-lookup.c:1569" "" { xfail *-*-* } 0 }
> +// { dg-bogus "x1template.h:18:13: internal compiler error: in resume_scope, at cp/name-lookup.c:1568" "" { xfail *-*-* } 0 }
> // { dg-prune-output "In file included from " }
>
> #include "x1template.h"
>
> --
> This patch is available for review at http://codereview.appspot.com/4634071
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pph] Fix binding_level's names_size streaming (issue4634071)
2011-06-23 2:20 ` Gabriel Charette
@ 2011-06-23 4:55 ` Gabriel Dos Reis
2011-06-23 11:33 ` Diego Novillo
0 siblings, 1 reply; 9+ messages in thread
From: Gabriel Dos Reis @ 2011-06-23 4:55 UTC (permalink / raw)
To: Gabriel Charette; +Cc: reply, gcc-patches, Diego Novillo, Lawrence Crowl
On Wed, Jun 22, 2011 at 7:05 PM, Gabriel Charette <gchare@google.com> wrote:
> And it looks like this wasn't sent to anyone directly...
> Adding back dnovillo and crowl (Diego I don't think Jason was ever
> added to the original message...?)
should not this go to mainline too?
-- Gaby
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pph] Fix binding_level's names_size streaming (issue4634071)
2011-06-23 4:55 ` Gabriel Dos Reis
@ 2011-06-23 11:33 ` Diego Novillo
2011-06-24 18:29 ` Gabriel Charette
0 siblings, 1 reply; 9+ messages in thread
From: Diego Novillo @ 2011-06-23 11:33 UTC (permalink / raw)
To: Gabriel Dos Reis
Cc: Gabriel Charette, reply, gcc-patches, Lawrence Crowl, Jason Merrill
On Wed, Jun 22, 2011 at 20:17, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
> On Wed, Jun 22, 2011 at 7:05 PM, Gabriel Charette <gchare@google.com> wrote:
>> And it looks like this wasn't sent to anyone directly...
>> Adding back dnovillo and crowl (Diego I don't think Jason was ever
>> added to the original message...?)
>
> should not this go to mainline too?
Yes, I CC'd Jason in the original thread that started this discussion.
Gab, could you send a patch for trunk? Please CC Jason when you do.
Diego.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pph] Fix binding_level's names_size streaming (issue4634071)
2011-06-23 11:33 ` Diego Novillo
@ 2011-06-24 18:29 ` Gabriel Charette
0 siblings, 0 replies; 9+ messages in thread
From: Gabriel Charette @ 2011-06-24 18:29 UTC (permalink / raw)
To: Diego Novillo
Cc: Gabriel Dos Reis, reply, gcc-patches, Lawrence Crowl, Jason Merrill
This was commited to trunk. Diego can you commit this patch to pph as well?
Thanks,
Gab
On Thu, Jun 23, 2011 at 4:07 AM, Diego Novillo <dnovillo@google.com> wrote:
> On Wed, Jun 22, 2011 at 20:17, Gabriel Dos Reis
> <gdr@integrable-solutions.net> wrote:
>> On Wed, Jun 22, 2011 at 7:05 PM, Gabriel Charette <gchare@google.com> wrote:
>>> And it looks like this wasn't sent to anyone directly...
>>> Adding back dnovillo and crowl (Diego I don't think Jason was ever
>>> added to the original message...?)
>>
>> should not this go to mainline too?
>
> Yes, I CC'd Jason in the original thread that started this discussion.
> Gab, could you send a patch for trunk? Please CC Jason when you do.
>
>
> Diego.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pph] Fix binding_level's names_size streaming (issue4634071)
@ 2011-06-24 20:05 dnovillo
0 siblings, 0 replies; 9+ messages in thread
From: dnovillo @ 2011-06-24 20:05 UTC (permalink / raw)
To: gchare, crowl, gdr; +Cc: gcc-patches, reply
On 2011/06/24 17:35:51, Gabriel Charette wrote:
> This was commited to trunk. Diego can you commit this patch to pph as
well?
Done. r175387.
Diego.
http://codereview.appspot.com/4634071/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pph] Fix binding_level's names_size streaming (issue4634071)
@ 2011-06-23 0:06 Gabriel Charette
0 siblings, 0 replies; 9+ messages in thread
From: Gabriel Charette @ 2011-06-23 0:06 UTC (permalink / raw)
To: reply, gcc-patches
Here is the patch removing names_size.
We found out it was write-only and that we could remove it.
I tested it with a full bootstrap build as well as a full regression test (make check-g++).
(actually one of the pph test fails, but that's because of a line number issue in where
we expect the ICE... I'll have to re-upload the patch as the script already submitted the files...)
2011-06-22 Gabriel Charette <gchare@google.com>
* gcc/cp/name-lookup.h (cp_binding_level): Removed unused
member names_size. Update all users.
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 54977ce..297c57e 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -544,7 +544,6 @@ add_decl_to_level (tree decl, cxx_scope *b)
necessary. */
TREE_CHAIN (decl) = b->names;
b->names = decl;
- b->names_size++;
/* If appropriate, add decl to separate list of statics. We
include extern variables because they might turn out to be
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 009b5d9..5f266eb 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -191,9 +191,6 @@ struct GTY(()) cp_binding_level {
are wrapped in TREE_LISTs; the TREE_VALUE is the OVERLOAD. */
tree names;
- /* Count of elements in names chain. */
- size_t names_size;
-
/* A chain of NAMESPACE_DECL nodes. */
tree namespaces;
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 0e8c6bf..a535cab 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -442,7 +442,6 @@ pph_in_binding_level (pph_stream *stream)
ALLOC_AND_REGISTER (stream, ix, bl, ggc_alloc_cleared_cp_binding_level ());
bl->names = pph_in_chain (stream);
- bl->names_size = pph_in_uint (stream);
bl->namespaces = pph_in_chain (stream);
bl->static_decls = pph_in_tree_vec (stream);
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index f219cef..7a6c516 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -448,7 +448,6 @@ pph_out_binding_level (pph_stream *stream, struct cp_binding_level *bl,
return;
pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS);
- pph_out_uint (stream, bl->names_size);
pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS);
pph_out_tree_vec (stream, bl->static_decls, ref_p);
--
This patch is available for review at http://codereview.appspot.com/4634071
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pph] Fix binding_level's names_size streaming (issue4634071)
@ 2011-06-21 1:39 Gabriel Charette
2011-06-22 18:36 ` Diego Novillo
0 siblings, 1 reply; 9+ messages in thread
From: Gabriel Charette @ 2011-06-21 1:39 UTC (permalink / raw)
To: reply, crowl, dnovillo, gcc-patches
We were streaming out the original names_size which includes the
built-in names which are not streamed out.
This only streams out the actual number of streamed names as names_size.
It appears names_size is not even used anywhere in the code
(or at least I couldn't find any use of it with `grep "names_size" -R gcc/`.
Should we just remove it?
This doesn't fix any currently failing pph tests.
Tested with bootstrap build and pph regression testing.
2011-06-20 Gabriel Charette <gchare@google.com>
* gcc/cp/pph-streamer-out.c (pph_count_filter_match): Added.
(pph_out_chain_filtered): Use pph_count_filter_match.
(pph_out_binding_level): Use pph_count_filter_match.
(pph_out_lang_specific): Fixed space typo.
(pph_write_tree): Fixed space typo.
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 49847a9..1f9ae1d 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -383,6 +383,34 @@ pph_out_label_binding (pph_stream *stream, cp_label_binding *lb, bool ref_p)
}
+/* Returns the number of nodes matching FILTER in chain
+ starting with FIRST. */
+
+static unsigned
+pph_count_filter_match (tree first, enum chain_filter filter)
+{
+ unsigned count;
+ tree t;
+
+ switch (filter)
+ {
+ case NO_BUILTINS:
+ for (t = first, count = 0; t; t = TREE_CHAIN (t))
+ {
+ if (filter == NO_BUILTINS && DECL_P (t) && DECL_IS_BUILTIN (t))
+ continue;
+ count++;
+ }
+ break;
+
+ default:
+ internal_error ("unknown chain_filter used in pph_count_filter_match");
+ }
+
+ return count;
+}
+
+
/* Output a chain of nodes to STREAM starting with FIRST. Skip any
nodes that do not match FILTER. REF_P is true if nodes in the chain
should be emitted as references. */
@@ -402,13 +430,7 @@ pph_out_chain_filtered (pph_stream *stream, tree first, bool ref_p,
return;
}
- /* Count all the nodes that match the filter. */
- for (t = first, count = 0; t; t = TREE_CHAIN (t))
- {
- if (filter == NO_BUILTINS && DECL_P (t) && DECL_IS_BUILTIN (t))
- continue;
- count++;
- }
+ count = pph_count_filter_match (first, filter);
pph_out_uint (stream, count);
/* Output all the nodes that match the filter. */
@@ -439,7 +461,7 @@ static void
pph_out_binding_level (pph_stream *stream, struct cp_binding_level *bl,
bool ref_p)
{
- unsigned i;
+ unsigned i, streamed_names_size;
cp_class_binding *cs;
cp_label_binding *sl;
struct bitpack_d bp;
@@ -448,7 +470,8 @@ pph_out_binding_level (pph_stream *stream, struct cp_binding_level *bl,
return;
pph_out_chain_filtered (stream, bl->names, ref_p, NO_BUILTINS);
- pph_out_uint (stream, bl->names_size);
+ streamed_names_size = pph_count_filter_match (bl->names, NO_BUILTINS);
+ pph_out_uint (stream, streamed_names_size);
pph_out_chain_filtered (stream, bl->namespaces, ref_p, NO_BUILTINS);
pph_out_tree_vec (stream, bl->static_decls, ref_p);
@@ -714,7 +737,7 @@ pph_out_lang_specific (pph_stream *stream, tree decl, bool ref_p)
ld = DECL_LANG_SPECIFIC (decl);
if (!pph_start_record (stream, ld))
return;
-
+
/* Write all the fields in lang_decl_base. */
ldb = &ld->u.base;
pph_out_ld_base (stream, ldb);
@@ -1080,7 +1103,7 @@ pph_write_tree (struct output_block *ob, tree expr, bool ref_p)
TI_TYPEDEFS_NEEDING_ACCESS_CHECKING (expr), ref_p);
break;
- case TEMPLATE_PARM_INDEX:
+ case TEMPLATE_PARM_INDEX:
{
template_parm_index *p = TEMPLATE_PARM_INDEX_CAST (expr);
pph_out_tree_common (stream, expr, ref_p);
--
This patch is available for review at http://codereview.appspot.com/4634071
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pph] Fix binding_level's names_size streaming (issue4634071)
2011-06-21 1:39 Gabriel Charette
@ 2011-06-22 18:36 ` Diego Novillo
0 siblings, 0 replies; 9+ messages in thread
From: Diego Novillo @ 2011-06-22 18:36 UTC (permalink / raw)
To: Gabriel Charette; +Cc: reply, crowl, gcc-patches
On Mon, Jun 20, 2011 at 21:12, Gabriel Charette <gchare@google.com> wrote:
> It appears names_size is not even used anywhere in the code
> (or at least I couldn't find any use of it with `grep "names_size" -R gcc/`.
> Should we just remove it?
Yes, we should remove it.
Jason, the field cp_binding_level.names_size is essentially write-only:
$ grep -r '\<names_size\>' .
./ChangeLog-2002: names_size and vtables.
./ChangeLog-2002: (wrapup_globals_for_namespace): Use names_size instead
./cp/ChangeLog-2002: names_size and vtables.
./cp/ChangeLog-2002: (wrapup_globals_for_namespace): Use names_size instead
./cp/name-lookup.h: size_t names_size;
./cp/name-lookup.c: b->names_size++;
I suppose it's OK to remove it, right?
Diego.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-06-24 19:05 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-23 0:17 [pph] Fix binding_level's names_size streaming (issue4634071) Gabriel Charette
2011-06-23 2:20 ` Gabriel Charette
2011-06-23 4:55 ` Gabriel Dos Reis
2011-06-23 11:33 ` Diego Novillo
2011-06-24 18:29 ` Gabriel Charette
-- strict thread matches above, loose matches on Subject: below --
2011-06-24 20:05 dnovillo
2011-06-23 0:06 Gabriel Charette
2011-06-21 1:39 Gabriel Charette
2011-06-22 18:36 ` 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).