Author: Bill Maddox Date: To: Kenneth Zadeck, Diego Novillo CC: Ollie Wild, Hubicha, Jan, gcc-patches, Kai Tietz Subject: Re: [lto]: merge from trunk
On Mon, Jul 14, 2008 at 12:06 AM, Bill Maddox <maddox@???> wrote: > I've done some analysis of this bug. The failure appears to be caused
> by the following lines in builtings.c (std_canonical_list_type):
>
> wtype = va_list_type_node;
> htype = type;
> /* ... */
> if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype))
> return va_list_type_node;
>
> Here, we are expecting the "well-known" type va_list_type_node,
> stashed away in a global variable during compiler initialization by
> both cc1 and lto1, to share substructure (the TYPE_MAIN_VARIANT) with
> that of a type that has been internalized from a stream.
Here is a patch that corrects this problem. It reorganizes the preloading of
well-known nodes in the streamer, and preloads the TYPE_MAIN_VARIANT of
well-known type nodes as well. It is a step on the way to proper sharing of
all of the node graph subsidiary to the well-known nodes.
The patch is mostly due to Diego, however, he was unable to complete it
before travelling, and handed it off to me to finish debugging. This should
correct the many assertion failures we've been seeing in stabilize_va_list
since a recent merge with mainline.
gcc:
* tree-pretty-print.c (dump_generic_node): Suppress display of
structure fields with TDF_SLIM flag.
fields even if structure is named.
(print_struct_decl): Test correctly for direct self-reference
or self-reference via a pointer type.
* tree.c (free_lang_specifics): Set ptrdiff_type_node to
integer_type_node, independent of the front-end.
* lto-function-out.c (output_tree): Improve streamer tracing.
Factor out code for keeping hash table of streamed nodes for
sharing purposes, moved to new function get_ref_idx_for.
* lto-function-in.c (input_tree_operand): Improve streamer tracing.
* lto-tree-out.h (struct output_block): Remove field next_main_index.
* tree-flow.h (tree_node_can_be_shared): Declare.
* tree-cfg.c (tree_node_can_be_shared): Make non-static.
* lto-section-out.c: Remove unnecessary header file inclusions.
(get_ref_idx_for): New function.
(preload_common_node): New version can be shared by reader and writer,
also preloads TYPE_MAIN_VARIANT field of preloaded types.
(preload_common_nodes): Use new preload_common_node. New tracing code.
* lto-section-out.h (preload_common_node, get_ref
gcc/lto:
* lto.c: Reorder file inclusions. Include lto-section-out.h.
(preload_common_nodes): Use new shared preload_common_node.
Improved streamer tracing.
* Make-lang.in: Make lto.o depend on lto-section-out.h.
if (TYPE_NAME (node))
dump_generic_node (buffer, TYPE_NAME (node), spc, flags, false);
- else
+ else if (!(flags & TDF_SLIM))
+ /* FIXME: If we eliminate the 'else' above and attempt
+ to show the fields for named types, we may get stuck
+ following a cycle of pointers to structs. The alleged
+ self-reference check in print_struct_decl will not detect
+ cycles involving more than one pointer or struct type. */
print_struct_decl (buffer, node, spc, flags);
break;
}
@@ -2364,8 +2369,8 @@ print_struct_decl (pretty_printer *buffe
Maybe this could be solved by looking at the scope in which the
structure was declared. */
if (TREE_TYPE (tmp) != node
- || (TREE_CODE (TREE_TYPE (tmp)) == POINTER_TYPE
- && TREE_TYPE (TREE_TYPE (tmp)) != node))
+ && (TREE_CODE (TREE_TYPE (tmp)) != POINTER_TYPE
+ || TREE_TYPE (TREE_TYPE (tmp)) != node))
{
print_declaration (buffer, tmp, spc+2, flags);
pp_newline (buffer);
Index: gcc/tree.c
===================================================================
--- gcc/tree.c (revision 138093)
+++ gcc/tree.c (working copy)
@@ -4004,6 +4004,7 @@ free_lang_specifics (void)
{
htab_traverse (uid2type_map, reset_type_lang_specific, NULL);
htab_traverse (decl_for_uid_map, reset_lang_specific, NULL);
+ ptrdiff_type_node = integer_type_node;
}
/* Return nonzero if IDENT is a valid name for attribute ATTR,
Index: gcc/lto-function-out.c
===================================================================
--- gcc/lto-function-out.c (revision 138093)
+++ gcc/lto-function-out.c (working copy)
@@ -2953,7 +2953,8 @@ output_tree (struct output_block *ob, tr
}
/* Map global decls and types to indices in the main stream. */
htab_t main_hash_table;
-
- /* Index in main stream of next node. */
- unsigned int next_main_index;
};
/* The global tree for the main identifier is filled in by language-specific
front-end initialization that is not run in the LTO back-end. It appears
@@ -119,28 +120,38 @@ preload_common_nodes (struct data_in *da
if (!main_identifier_node)
main_identifier_node = get_identifier ("main");
-/* Assign an index to tree node T and enter it in the global streamer
- hash table OB->MAIN_HASH_TABLE. */
+/* Return a reference index for tree node T from hash table H. If
+ node T does not already exist in table H, a new entry is created
+ for it. The returned reference is used to identify multiple
+ instances of T on the file stream. This is used in two ways:
+
+ - On the writing side, the first time T is added to H, a new reference
+ index is created for T and T is emitted on the stream. If T
+ needs to be emitted again to the stream, instead of pickling it
+ again, the reference index is emitted.
+
+ - On the reading side, the first time T is read from the stream,
+ it is reconstructed in memory and a new reference index created
+ for T. The reconstructed T is inserted in some array so that
+ when the reference index for T is found in the input stream, it
+ can be used to look up into the array to get the reconstructed T.
-static void
-preload_common_node (struct output_block *ob, tree t)
+ *NEXT_REF_P points to the next available reference index to be
+ handed out. If T is inserted into H, this value is updated.
+
+ The reference index for T is returned in *REF_P. The function
+ returns true if T was already in H. Otherwise, it returns false. */
+
+bool
+get_ref_idx_for (tree t, htab_t h, VEC(tree, heap) **v, unsigned *ref_p)
{
void **slot;
struct lto_decl_slot d_slot;
+ unsigned next_ref_idx = htab_elements (h);
+ bool retval;
+ retval = true;
d_slot.t = t;
- slot = htab_find_slot (ob->main_hash_table, &d_slot, INSERT);
-
- /* If well-known trees are not unique, we don't create duplicate entries. */
+ slot = htab_find_slot (h, &d_slot, INSERT);
if (*slot == NULL)
{
- struct lto_decl_slot *new_slot
- = (struct lto_decl_slot *) xmalloc (sizeof (struct lto_decl_slot));
- unsigned index = ob->next_main_index++;
+ struct lto_decl_slot *new_slot = XNEW (struct lto_decl_slot);
new_slot->t = t;
- new_slot->slot_num = index;
+ new_slot->slot_num = next_ref_idx;
*slot = new_slot;
+
+ if (v)
+ {
+ gcc_assert (next_ref_idx == VEC_length (tree, *v));
+ VEC_safe_push (tree, heap, *v, t);
+ }
+
+ /* Indicate that the item was not in the table before by
+ returning false. */
+ retval = false;
+ }
+
+ if (ref_p)
+ *ref_p = ((struct lto_decl_slot *) *slot)->slot_num;
+
+ return retval;
+}
+
+
+/* Assign an index to tree node T and enter it in the global streamer
+ hash table OB->MAIN_HASH_TABLE. */
+
+void
+preload_common_node (tree t, htab_t h, VEC(tree, heap) **v, unsigned *ref_p)
+{
+ /* Skip empty slots. Note that we will be in trouble if
+ the empty slots do not match on both the writer and reader side. */
+ if (!t) return;
+
#ifdef GLOBAL_STREAMER_TRACE
- fprintf (stderr, "preloaded 0x%x: ", index);
- print_generic_expr (stderr, t, 0);
- fprintf (stderr, "\n");
+ fprintf (stderr, "Preloading common node: [%s] ",
+ tree_code_name[TREE_CODE (t)]);
+ print_generic_expr (stderr, t, 0);
+ fprintf (stderr, "\n");
#endif
+
+ if (get_ref_idx_for (t, h, v, ref_p))
+ return;
+
+ /* FIXME: In principle, we should perform a walk over all nodes reachable
+ from each preloaded node. This is going to be a lot of work. At present,
+ we catch the case that was causing test failures. A small step. */
+ if (tree_node_can_be_shared (t))
+ {
+ if (TYPE_P (t))
+ {
+ get_ref_idx_for (TYPE_MAIN_VARIANT (t), h, v, ref_p);
+ }
}
- else
- /* Skip the index, which will leave an unused slot in the
- globals vector in the reader. Otherwise, the reader
- initialization must perform a similar duplicate-removal
- process to reconstruct a valid vector. NOTE: This isn't
- difficult. Perhaps we should just do it. */
- ob->next_main_index++;
}
