This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [lto] delay function materialization
- From: Kenneth Zadeck <zadeck at naturalbridge dot com>
- To: gcc-patches at gcc dot gnu dot org, zadeck at naturalbridge dot com, "Hubicha, Jan" <jh at suse dot cz>
- Date: Mon, 22 Oct 2007 23:16:17 -0400
- Subject: Re: [lto] delay function materialization
- References: <20071022222841.GG3425@codesourcery.com>
Nathan Froyd wrote:
> The below patch fixes an issue with reading struct fields and an issue
> with cgraph. We would occasionally get messages like:
>
> preprocessed-gcc/global.o:0: sorry, unimplemented: member declaration not inside structure, class, or union
>
> The DWARF we would be looking at would be something like:
>
> <2><d5fe>: Abbrev Number: 36 (DW_TAG_structure_type)
> <d5ff> DW_AT_byte_size : 8
> <d600> DW_AT_decl_file : 1
> <d601> DW_AT_decl_line : 377
> <d603> DW_AT_sibling : <d626>
> ...
> <3><d617>: Abbrev Number: 8 (DW_TAG_member)
> <d618> DW_AT_name : to
> <d61b> DW_AT_decl_file : 1
> <d61c> DW_AT_decl_line : 377
> <d61e> DW_AT_type : <52d>
> <d622> DW_AT_data_member_location: 2 byte block: 23 4 (DW_OP_plus_uconst: 4)
>
> And we would be asked for the FIELD_DECL corresponding to DIE 0xd617.
> The problem is that we would attempt to read the DIE directly (usually
> through a call chain like lto_read_function_body -> lto_read_body ->
> input_globals -> lto_resolve_field_ref -> lto_read_DIE_at_ptr), ignoring
> that it is meant to be read in a particular context, i.e. that of its
> parent structure. When reading the field, we'd assert that we were in
> such a context and fall over.
>
> We could have fixed this by just removing the assert, since we really
> did want just the field in this instance. I think this would have
> eventually caused problems, since the field would not be "attached"
> properly to its containing structure. The tack I took instead was to
> delay reading in function bodies ("materialization") until we have read
> all of the DWARF info. We therefore ensure that we read the DWARF in
> the manner expected--tree-like--and that fields are properly attached to
> their parents, etc. I think we wanted this for other reasons, but
> fixing the above problem and reading DWARF properly seem like good
> reasons to me.
>
> The other bug this patch fixes is that we were not calling
> init_ssa_operands like cgraph expected us to, so deep within cgraph,
> we'd call fini_ssa_operands, which eventually led to problems after
> several finalizations without the corresponding initializations. It
> doesn't seem to matter whether we call init_ssa_operands before we read
> in the function body or after, so I attempted to pick the most obvious
> place.
>
This seems to cause a huge number of crashes of the form:
Honza do we need some stripped down version of this or something?
kenny
Program received signal SIGSEGV, Segmentation fault.
0x00000000008ca74d in init_ssa_operands ()
at ../../gccLTO/gcc/tree-ssa-operands.c:374
374 gcc_assert (gimple_ssa_operands (cfun)->operand_memory == NULL);
(gdb) bt
#0 0x00000000008ca74d in init_ssa_operands ()
at ../../gccLTO/gcc/tree-ssa-operands.c:374
#1 0x000000000041e87a in lto_read_body (fd=0x12d6f50,
context=0x7fffb2c877d0,
t=0x2b70f7efb000, data=0x12dcc90, in_function=1 '\001')
at ../../gccLTO/gcc/lto/lto-read.c:1629
#2 0x000000000041ea37 in lto_read_function_body (fd=0x12d6f50,
context=0x7fffb2c877d0, fn_decl=0x2b70f7efb000, data=0x12dcc90)
at ../../gccLTO/gcc/lto/lto-read.c:1693
#3 0x0000000000412d7a in lto_materialize_function (fd=0x12d6f50,
context=0x7fffb2c877d0, decl=0x2b70f7efb000)
at ../../gccLTO/gcc/lto/lto.c:2418
#4 0x000000000041527a in lto_file_read (file=0x12d6f40)
at ../../gccLTO/gcc/lto/lto.c:3408
#5 0x0000000000415654 in lto_main (debug_p=0)
at ../../gccLTO/gcc/lto/lto.c:3549
#6 0x000000000074d004 in compile_file () at ../../gccLTO/gcc/toplev.c:1046
#7 0x000000000074ece9 in do_compile () at ../../gccLTO/gcc/toplev.c:2246
#8 0x000000000074ed4d in toplev_main (argc=5, argv=0x7fffb2c879d8)
at ../../gccLTO/gcc/toplev.c:2278
#9 0x0000000000423a5b in main (argc=5, argv=0x7fffb2c879d8)
at ../../gccLTO/gcc/main.c:35
(gdb) run
> Committed to the lto branch.
>
> -Nathan
>
> 2007-10-22 Nathan Froyd <froydnj@codesourcery.com>
>
> * lto.h (struct lto_info_fd): Add field unmaterialized_fndecls.
> * lto.c (lto_info_fd_init): Initialize it.
> (lto_info_fd_close): Free it.
> (lto_materialize_function): New function.
> (lto_read_subroutine_type_subprogram_DIE): Save the result decl on
> unmaterialized_fndecls.
> (lto_file_read): Read in all the function bodies after we have read
> all of the DWARF info.
> * lto-read.c (lto_read_body): Call init_ssa_operands if we are
> reading a function body.
>
> Index: lto.c
> ===================================================================
> --- lto.c (revision 129550)
> +++ lto.c (working copy)
> @@ -247,6 +247,7 @@ lto_info_fd_init (lto_info_fd *fd, const
> lto_cache_hash,
> lto_cache_eq,
> free);
> + fd->unmaterialized_fndecls = VEC_alloc (tree, heap, 32);
> }
>
> /* Initialize FD, a newly allocated file descriptor for a DWARF2
> @@ -269,6 +270,10 @@ lto_info_fd_close (lto_info_fd *fd)
> for (i = 0; i < fd->num_units; ++i)
> XDELETE (fd->units[i]);
> XDELETEVEC (fd->units);
> +
> + /* Other people are holding references to the trees contained in this,
> + so just free the vector. */
> + VEC_free (tree, heap, fd->unmaterialized_fndecls);
> }
>
> /* Close FD. */
> @@ -2388,6 +2393,42 @@ lto_read_abbrev (lto_info_fd *fd)
> return abbrev;
> }
>
> +/* Read the function body for DECL out of FD if possible. */
> +
> +static void
> +lto_materialize_function (lto_info_fd *fd,
> + lto_context *context,
> + tree decl)
> +{
> + lto_file *file;
> + const void *body;
> + const char *name;
> +
> + file = fd->base.file;
> + name = IDENTIFIER_POINTER (DECL_NAME (decl));
> + body = file->vtable->map_fn_body (file, name);
> +
> + if (body)
> + {
> + /* This function has a definition. */
> + TREE_STATIC (decl) = 1;
> + DECL_EXTERNAL (decl) = 0;
> +
> + allocate_struct_function (decl);
> + lto_read_function_body (fd, context, decl, body);
> + file->vtable->unmap_fn_body (file, name, body);
> + }
> + else
> + DECL_EXTERNAL (decl) = 0;
> +
> + /* Let the middle end know about the function. */
> + rest_of_decl_compilation (decl,
> + /*top_level=*/1,
> + /*at_end=*/0);
> + if (body)
> + cgraph_finalize_function (decl, /*nested=*/false);
> +}
> +
> static tree
> lto_read_subroutine_type_subprogram_DIE (lto_info_fd *fd,
> lto_die_ptr die ATTRIBUTE_UNUSED,
> @@ -2541,27 +2582,11 @@ lto_read_subroutine_type_subprogram_DIE
> result = type;
> else
> {
> - const void *body;
> - lto_file *file;
> - const char *name_str;
> -
> if (!name)
> lto_file_corrupt_error ((lto_fd *)fd);
>
> result = build_decl (FUNCTION_DECL, name, type);
> TREE_PUBLIC (result) = external;
> - /* Load the body of the function. */
> - file = fd->base.file;
> - name_str = IDENTIFIER_POINTER (name);
> - body = file->vtable->map_fn_body (file, name_str);
> - if (body)
> - {
> - /* This function has a definition. */
> - TREE_STATIC (result) = 1;
> - DECL_EXTERNAL (result) = 0;
> - }
> - else
> - DECL_EXTERNAL (result) = 1;
> if (inlined == DW_INL_declared_inlined
> || inlined == DW_INL_declared_not_inlined)
> DECL_DECLARED_INLINE_P (result) = 1;
> @@ -2574,27 +2599,18 @@ lto_read_subroutine_type_subprogram_DIE
> declarations. */
> result = lto_symtab_merge_fn (result);
> if (result != error_mark_node)
> - {
> - /* Record this function in the DIE cache so that it can be
> - resolved from the bodies of functions. */
> - lto_cache_store_DIE (fd, die, result);
> - /* If the function has a body, read it in. */
> - if (body)
> - {
> - DECL_RESULT (result)
> - = build_decl (RESULT_DECL, NULL_TREE,
> - TYPE_MAIN_VARIANT (ret_type));
> - allocate_struct_function (result);
> - lto_read_function_body (fd, context, result, body);
> - file->vtable->unmap_fn_body (file, name_str, body);
> - }
> - /* Let the middle end know about the function. */
> - rest_of_decl_compilation (result,
> - /*top_level=*/1,
> - /*at_end=*/0);
> - if (body)
> - cgraph_finalize_function (result, /*nested=*/false);
> - }
> + {
> + /* Record this function in the DIE cache so that it can be
> + resolved from the bodies of functions. */
> + lto_cache_store_DIE (fd, die, result);
> +
> + DECL_RESULT (result)
> + = build_decl (RESULT_DECL, NULL_TREE,
> + TYPE_MAIN_VARIANT (ret_type));
> + }
> +
> + /* Save it for later. */
> + VEC_safe_push (tree, heap, fd->unmaterialized_fndecls, result);
> }
>
> /* Read the child DIEs, which are in the scope of RESULT. */
> @@ -3365,6 +3381,8 @@ lto_file_read (lto_file *file)
> DWARF2_CompUnit *unit = file->debug_info.units[i];
> /* The context information for this compilation unit. */
> lto_context context;
> + size_t j;
> + tree decl;
>
> /* Set up the context. */
> lto_set_cu_context (&context, &file->debug_info, unit);
> @@ -3380,6 +3398,14 @@ lto_file_read (lto_file *file)
> /* Read DIEs. */
> while (fd->cur < context.cu_end)
> lto_read_DIE (&file->debug_info, &context, NULL);
> +
> + /* Read in function bodies now that we have the full DWARF tree
> + available. */
> + for (j = 0;
> + VEC_iterate (tree, file->debug_info.unmaterialized_fndecls,
> + j, decl);
> + j++)
> + lto_materialize_function (&file->debug_info, &context, decl);
> }
>
> return true;
> Index: lto.h
> ===================================================================
> --- lto.h (revision 129550)
> +++ lto.h (working copy)
> @@ -26,6 +26,7 @@ Boston, MA 02110-1301, USA. */
>
> #include "hashtab.h"
> #include "tree.h"
> +#include "vec.h"
> #include <inttypes.h>
>
> /* Forward Declarations */
> @@ -66,6 +67,8 @@ typedef struct lto_info_fd
> /* A map from DIEs to trees. The keys are offsets into the DWARF
> information section; the values are trees. */
> htab_t die_cache;
> + /* A vector of FUNCTION_DECLs whose bodies have not yet been read in. */
> + VEC (tree, heap) *unmaterialized_fndecls;
> } lto_info_fd;
>
> /* A file descriptor for reading from a DWARF abbreviation section. */
> Index: lto-read.c
> ===================================================================
> --- lto-read.c (revision 129550)
> +++ lto-read.c (working copy)
> @@ -1623,6 +1623,9 @@ lto_read_body (lto_info_fd *fd,
> cfun = fn;
> data_in.num_named_labels = header->num_named_labels;
>
> + /* cgraph expects this to be called once for each function. */
> + init_ssa_operands ();
> +
> #ifdef LTO_STREAM_DEBUGGING
> lto_debug_context.current_data = &debug_label;
> #endif
>