[pph] Fix some cgraph node merge problems (issue5302068)
Diego Novillo
dnovillo@google.com
Thu Oct 27 05:04:00 GMT 2011
This patch fixes some of the problems we had with cgraph nodes for
merged functions. When merging a function that had a cgraph node emitted
for it, we were ICEing during cgraph allocation because the reader
was not checking whether the node already existed for that function.
Additionally, the patch removes the computation of hash values for
include branches. That was only papering over hashing problems.
Instead, I made the hashing key more descriptive by using
decl_as_string and the mangled name.
We still have several merging problems. The x4tmplfun* and z4tmplfun*
failures are different ICEs in template instantiation. Still have not
looked what those are.
Tested on x86_64. Committed.
Diego.
* cp-tree.h (get_mangled_id): Declare.
* mangle.c (get_mangled_id): Factor out of ...
(mangle_decl): ... here.
* pph-streamer-in.c
(pph_get_include_path_hash): Remove. Update all users.
* pph-streamer-out.c (pph_merge_name): Move from pph-streamer.c.
Change return type to char *.
Call get_mangled_id and decl_as_string.
Update users.
testsuite/ChangeLog.pph
* g++.dg/pph/c4inline.cc: Mark fixed.
* g++.dg/pph/x4tmplclass1.cc: Likewise.
* g++.dg/pph/x4tmplclass2.cc: Likewise.
* g++.dg/pph/z4tmplclass1.cc: Likewise.
* g++.dg/pph/z4tmplclass2.cc: Likewise.
* g++.dg/pph/x4tmplfuncinln.cc: Change expected failure.
* g++.dg/pph/x4tmplfuncninl.cc: Likewise.
* g++.dg/pph/z4tmplfuncinln.cc: Likewise.
* g++.dg/pph/z4tmplfuncninl.cc: Likewise.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 705c0a6..297a779 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5782,6 +5782,7 @@ extern tree merge_exception_specifiers (tree, tree, tree);
/* in mangle.c */
extern void init_mangle (void);
extern void mangle_decl (tree);
+extern tree get_mangled_id (tree);
extern const char *mangle_type_string (tree);
extern tree mangle_typeinfo_for_type (tree);
extern tree mangle_typeinfo_string_for_type (tree);
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 55851e6..38a1fcb 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -3157,13 +3157,21 @@ mangle_decl_string (const tree decl)
return result;
}
+/* Return an identifier for the external mangled name of DECL. */
+
+tree
+get_mangled_id (tree decl)
+{
+ tree id = mangle_decl_string (decl);
+ return targetm.mangle_decl_assembler_name (decl, id);
+}
+
/* Create an identifier for the external mangled name of DECL. */
void
mangle_decl (const tree decl)
{
- tree id = mangle_decl_string (decl);
- id = targetm.mangle_decl_assembler_name (decl, id);
+ tree id = get_mangled_id (decl);
SET_DECL_ASSEMBLER_NAME (decl, id);
if (G.need_abi_warning)
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 6c8bec4..6aa301f 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -67,7 +67,7 @@ static VEC(char_p,heap) *string_tables = NULL;
static int pph_loc_offset;
/* Forward declarations to avoid circularity. */
-static tree pph_in_merge_key_tree (pph_stream *, tree *, hashval_t);
+static tree pph_in_merge_key_tree (pph_stream *, tree *);
/***************************************************** stream initialization */
@@ -690,18 +690,17 @@ pph_in_chain (pph_stream *stream)
/* Read a chain of AST merge keys from STREAM. Merge each tree
- into *CHAIN. IPATH_HASH is the hash value of the include path
- from STREAM to the root of the include tree. */
+ into *CHAIN. */
static void
-pph_in_merge_key_chain (pph_stream *stream, tree *chain, hashval_t ipath_hash)
+pph_in_merge_key_chain (pph_stream *stream, tree *chain)
{
unsigned i;
HOST_WIDE_INT count;
count = pph_in_hwi (stream);
for (i = 0; i < count; i++)
- pph_in_merge_key_tree (stream, chain, ipath_hash);
+ pph_in_merge_key_tree (stream, chain);
}
@@ -740,13 +739,6 @@ typedef struct {
/* Name of the tree (from pph_merge_name). */
const char *name;
-
- /* Hash value representing the include path starting at the image
- where EXPR resides up to the root of the include tree. Objects
- found in any of these PPH images do not need to be merged. They
- were already emitted as external references and merged when
- the PPH images were being generated. */
- hashval_t ipath_hash;
} merge_toc_entry;
@@ -767,11 +759,6 @@ htab_merge_key_eq (const void *p1, const void *p2)
const merge_toc_entry *key1 = (const merge_toc_entry *) p1;
const merge_toc_entry *key2 = (const merge_toc_entry *) p2;
- /* Matches inside the same include path are not interesting. These
- symbols have already been merged. */
- if (key1->ipath_hash == key2->ipath_hash)
- return false;
-
if (key1->context != key2->context)
return false;
@@ -831,15 +818,12 @@ pph_prepend_to_chain (tree expr, tree *chain)
}
-/* Merge the just-read header for tree EXPR with NAME onto the CHAIN.
- IPATH_HASH is the hash value of the include path from STREAM to the
- root of the include tree. */
+/* Merge the just-read header for tree EXPR with NAME onto the CHAIN. */
static tree
-pph_merge_into_chain (tree expr, const char *name, tree *chain,
- hashval_t ipath_hash)
+pph_merge_into_chain (tree expr, const char *name, tree *chain)
{
- merge_toc_entry key = { expr, chain, name, ipath_hash };
+ merge_toc_entry key = { expr, chain, name };
tree found = pph_toc_lookup (merge_toc, &key);
if (!found)
{
@@ -1893,11 +1877,10 @@ pph_in_tree_header (pph_stream *stream, enum LTO_tags tag)
/* Read a merge key from STREAM. If the merge key read from
STREAM is not found in *CHAIN, the newly allocated tree is added to
- it. IPATH_HASH is the hash value of the include path from STREAM to
- the root of the include tree. */
+ it. */
static tree
-pph_in_merge_key_tree (pph_stream *stream, tree *chain, hashval_t ipath_hash)
+pph_in_merge_key_tree (pph_stream *stream, tree *chain)
{
struct lto_input_block *ib = stream->encoder.r.ib;
enum pph_record_marker marker;
@@ -1918,7 +1901,7 @@ pph_in_merge_key_tree (pph_stream *stream, tree *chain, hashval_t ipath_hash)
/* Look for a match in CHAIN to READ_EXPR's header. If we found a
match, EXPR will be the existing tree that matches READ_EXPR.
Otherwise, EXPR is the newly allocated READ_EXPR. */
- expr = pph_merge_into_chain (read_expr, name, chain, ipath_hash);
+ expr = pph_merge_into_chain (read_expr, name, chain);
gcc_assert (expr != NULL);
@@ -2079,7 +2062,7 @@ pph_in_cgraph_node (pph_stream *stream)
fndecl = pph_in_tree (stream);
ALLOC_AND_REGISTER (&stream->cache, ix, PPH_cgraph_node, node,
- cgraph_create_node (fndecl));
+ cgraph_get_create_node (fndecl));
node->origin = pph_in_cgraph_node (stream);
node->nested = pph_in_cgraph_node (stream);
@@ -2330,22 +2313,6 @@ pph_in_identifiers (pph_stream *stream, cpp_idents_used *identifiers)
}
-/* Compute a hash value for all the images starting at STREAM up to the
- root of the include hierarchy. */
-
-static hashval_t
-pph_get_include_path_hash (pph_stream *stream)
-{
- pph_stream *i;
- hashval_t val;
-
- for (val = 0, i = stream; i; i = i->parent)
- val = iterative_hash_hashval_t (htab_hash_pointer (i), val);
-
- return val;
-}
-
-
/* Read all the merge keys from STREAM. Merge into the corresponding
contexts. Return a VEC of all the merge keys read. */
@@ -2353,19 +2320,12 @@ static void
pph_in_merge_keys (pph_stream *stream)
{
cp_binding_level *bl = scope_chain->bindings;
- hashval_t include_path_hash = 0;
-
- /* Compute the signature for the include path from STREAM up to
- the root of the inclusion tree. Symbols found in any image in
- the direct path from STREAM up to the root do not need to be
- merged. They were already merged when the images were generated. */
- include_path_hash = pph_get_include_path_hash (stream);
/* First read all the merge keys and merge into the global bindings. */
- pph_in_merge_key_chain (stream, &bl->names, include_path_hash);
- pph_in_merge_key_chain (stream, &bl->namespaces, include_path_hash);
- pph_in_merge_key_chain (stream, &bl->usings, include_path_hash);
- pph_in_merge_key_chain (stream, &bl->using_directives, include_path_hash);
+ pph_in_merge_key_chain (stream, &bl->names);
+ pph_in_merge_key_chain (stream, &bl->namespaces);
+ pph_in_merge_key_chain (stream, &bl->usings);
+ pph_in_merge_key_chain (stream, &bl->using_directives);
/* Now read the bodies of all the trees merged above. */
pph_in_merge_body_chain (stream);
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index ddff891..29d7462 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -1899,17 +1899,49 @@ pph_out_tree_header (pph_stream *stream, tree expr)
}
+/* Return the merge name string identifier tree for a decl EXPR. The
+ caller is responsible of freeing the returned string. */
+
+static char *
+pph_merge_name (tree expr)
+{
+ char *retval;
+ size_t len;
+ const char *mangled_str, *name_str;
+ unsigned flags = TFF_PLAIN_IDENTIFIER
+ | TFF_SCOPE
+ | TFF_DECL_SPECIFIERS
+ | TFF_CLASS_KEY_OR_ENUM
+ | TFF_RETURN_TYPE;
+
+
+ if (TREE_CODE (expr) == FUNCTION_DECL)
+ flags |= TFF_RETURN_TYPE
+ | TFF_FUNCTION_DEFAULT_ARGUMENTS
+ | TFF_EXCEPTION_SPECIFICATION;
+ else if (TREE_CODE (expr) == TEMPLATE_DECL)
+ flags |= TFF_TEMPLATE_HEADER
+ | TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS;
+
+ mangled_str = IDENTIFIER_POINTER (get_mangled_id (expr));
+ name_str = decl_as_string (expr, flags);
+
+ len = strlen (name_str) + sizeof("|") + strlen (mangled_str);
+ retval = XNEWVEC (char, len + 1);
+ sprintf (retval, "%s|%s", name_str, mangled_str);
+
+ return retval;
+}
+
+
/* Write the merge name string for a decl EXPR. */
static void
pph_out_merge_name (pph_stream *stream, tree expr)
{
- tree name = pph_merge_name (expr);
- if (name)
- pph_out_string_with_length (stream, IDENTIFIER_POINTER (name),
- IDENTIFIER_LENGTH (name));
- else
- pph_out_string (stream, NULL);
+ char *name = pph_merge_name (expr);
+ pph_out_string (stream, name);
+ free (name);
}
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 8624c13..9e23529 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -376,7 +376,10 @@ pph_cache_insert_at (pph_cache *cache, void *data, unsigned ix,
map_slot = pointer_map_insert (cache->m, data);
- /* We should not be trying to insert the same data more than once. */
+ /* We should not be trying to insert the same data more than once.
+ This indicates that the same DATA pointer has been given two
+ different cache locations. This almost always points to a
+ problem with merging data structures read from different files. */
gcc_assert (*map_slot == NULL);
*map_slot = (void *) (intptr_t) ix;
@@ -575,17 +578,3 @@ pph_get_signature (tree t, size_t *nbytes_p)
return crc;
}
-
-
-/* Return the merge name string identifier tree for a decl EXPR. */
-
-tree
-pph_merge_name (tree expr)
-{
- if (TREE_CODE (expr) == FUNCTION_DECL
- && !DECL_BUILT_IN (expr)
- && DECL_LANG_SPECIFIC (expr))
- return DECL_ASSEMBLER_NAME (expr);
- else
- return DECL_NAME (expr);
-}
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 35791bb..76eaa15 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -252,7 +252,6 @@ pph_cache_entry *pph_cache_add (pph_cache *, void *, unsigned *, enum pph_tag);
void pph_cache_sign (pph_cache *, unsigned, unsigned, size_t);
unsigned pph_get_signature (tree, size_t *);
void pph_writer_add_include (pph_stream *);
-tree pph_merge_name (tree expr);
/* In pph-streamer-out.c. */
void pph_flush_buffers (pph_stream *);
diff --git a/gcc/testsuite/g++.dg/pph/c4inline.cc b/gcc/testsuite/g++.dg/pph/c4inline.cc
index cd3df87..e1c76ff 100644
--- a/gcc/testsuite/g++.dg/pph/c4inline.cc
+++ b/gcc/testsuite/g++.dg/pph/c4inline.cc
@@ -1,6 +1,3 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "c4inline.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
#include "c0inline1.h"
#include "c0inline2.h"
diff --git a/gcc/testsuite/g++.dg/pph/x4tmplclass1.cc b/gcc/testsuite/g++.dg/pph/x4tmplclass1.cc
index 46d1f97..f82cf41 100644
--- a/gcc/testsuite/g++.dg/pph/x4tmplclass1.cc
+++ b/gcc/testsuite/g++.dg/pph/x4tmplclass1.cc
@@ -1,7 +1,4 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "x4tmplclass1.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
-// Previoiusly:
+// pph asm xdiff 63957
// Assembly differences seem to be due to the order in which the
// symbols in the template hash tables are emitted.
#include "x0tmplclass11.h"
diff --git a/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc b/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
index 0b36b27..975eb9a 100644
--- a/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
+++ b/gcc/testsuite/g++.dg/pph/x4tmplclass2.cc
@@ -1,11 +1,4 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "x4tmplclass2.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
-// Prevously:
-// xfail BOGUS DUPFUN
-// base<short>::dynamic_early_inline() and base<short>::static_early_inline()
-// are duplicated.
-
+// pph asm xdiff 28477
#include "x0tmplclass21.h"
#include "x0tmplclass22.h"
#include "a0tmplclass2_u.h"
diff --git a/gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc b/gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc
index 3115a70..8477bd0 100644
--- a/gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc
+++ b/gcc/testsuite/g++.dg/pph/x4tmplfuncinln.cc
@@ -1,6 +1,6 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "x4tmplfuncinln.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
+// { dg-xfail-if "ICE instantiate_decl - bad merge" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "internal compiler error: in instantiate_decl, at cp/pt.c" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "decl merge problems" { xfail *-*-* } }
#include "x0tmplfuncinln1.h"
#include "x0tmplfuncinln2.h"
#include "a0tmplfuncinln_u.h"
diff --git a/gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc b/gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
index 3daa98a..4d9f87e 100644
--- a/gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
+++ b/gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc
@@ -1,6 +1,6 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "x4tmplfuncninl.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
+// { dg-xfail-if "ICE instantiate_decl - bad merge" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "internal compiler error: in instantiate_decl, at cp/pt.c" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "decl merge problems" { xfail *-*-* } }
#include "x0tmplfuncninl1.h"
#include "x0tmplfuncninl2.h"
#include "a0tmplfuncninl_u.h"
diff --git a/gcc/testsuite/g++.dg/pph/z4tmplclass1.cc b/gcc/testsuite/g++.dg/pph/z4tmplclass1.cc
index d7844bd..45e23a8 100644
--- a/gcc/testsuite/g++.dg/pph/z4tmplclass1.cc
+++ b/gcc/testsuite/g++.dg/pph/z4tmplclass1.cc
@@ -1,7 +1,4 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "z4tmplclass1.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
-// Previously
+// pph asm xdiff 26829
// xfail BOGUS DUPVAR DUPFUNC
#include "x0tmplclass13.h"
diff --git a/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc b/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
index 1ae61b7..4d2bac9 100644
--- a/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
+++ b/gcc/testsuite/g++.dg/pph/z4tmplclass2.cc
@@ -1,7 +1,4 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "z4tmplclass2.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
-// Previously
+// pph asm xdiff 16961
// xfail BOGUS DUPVAR DUPFUNC
#include "x0tmplclass23.h"
diff --git a/gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc b/gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc
index 09dbbd1..6f9e18d 100644
--- a/gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc
+++ b/gcc/testsuite/g++.dg/pph/z4tmplfuncinln.cc
@@ -1,6 +1,6 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "z4tmplfuncinln.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
+// { dg-xfail-if "ICE instantiate_decl - bad merge" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "internal compiler error: in instantiate_decl, at cp/pt.c" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "decl merge problems" { xfail *-*-* } }
#include "x0tmplfuncinln3.h"
#include "x0tmplfuncinln4.h"
#include "a0tmplfuncinln_u.h"
diff --git a/gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc b/gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
index 644fe53..e33ad27 100644
--- a/gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
+++ b/gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc
@@ -1,6 +1,6 @@
-// { dg-xfail-if "ICE CGRAPH" { "*-*-*" } { "-fpph-map=pph.map" } }
-// { dg-bogus "z4tmplfuncninl.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502" "" { xfail *-*-* } 0 }
-
+// { dg-xfail-if "ICE instantiate_decl - bad merge" { "*-*-*" } { "-fpph-map=pph.map" } }
+// { dg-bogus "internal compiler error: in instantiate_decl, at cp/pt.c" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "decl merge problems" { xfail *-*-* } }
#include "x0tmplfuncninl3.h"
#include "x0tmplfuncninl4.h"
#include "a0tmplfuncninl_u.h"
--
1.7.3.1
--
This patch is available for review at http://codereview.appspot.com/5302068
More information about the Gcc-patches
mailing list