This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [lto] PATCH: DIE readers for struct/union/class types, + other bug fixes

Sandra Loosemore wrote:

2006-09-29 Sandra Loosemore <>

	* gcc/lto/lto.c (lto_check_HOST_WIDE_INT_val): New.
	(lto_read_form): Make it know about DW_AT_bit_offset, DW_AT_bit_size,
	DW_AT_data_member_location, DW_AT_specification.
	(struct lto_die_cache_entry): Add sibling field.
	(lto_cache_store_DIE): Set sibling field.
	(lto_cache_lookup_DIE): Add SKIP argument; if true, skip to stored
	sibling pointer when a cache hit is found.
	(lto_read_referenced_type_DIE): Save/restore context->parentdata
	while reading the DIE.  Add FIXME note about potential bugs.
	(lto_read_structure_union_class_type_DIE): New.
	(lto_read_member_DIE): New.
	(lto_read_pointer_reference_type_DIE): Treat missing type attribute
	as void instead of error, for compatibility with dwarf2out.c behavior.
	(lto_read_const_volatile_restrict_type_DIE): Likewise.
	(lto_read_DIE): Add entries for new DIE readers to lookup table.
	Don't read children of an already-processed DIE a second time.

This looks very good. A few nits:

+ 	  /* This is kind of nasty.  DWARF doesn't encode the overall
+ 	     alignment of the struct/union type, so we'll take a stab at
+ 	     recomputing it and hope it comes out the same as for the
+ 	     original type.  */
+ 	  if (DECL_ALIGN (child) > align)
+ 	    align = DECL_ALIGN (child);

This is OK for the moment, but I think we need a different long-term solution. We should either use the usual stor-layout.c routines (place_field, etc.), or we should encode everything in DWARF and then just completely bypass all of that stuff. I like the second alternative better because it eliminates language-dependent oddities.

So, I think we should take advantage of DWARF's extensibility to add additional attributes for the things we need. One such would be alignment of the record type.

+ 	  /* FIXME: mess with propagating mutable/volatile/etc attributes
+ 	     back to parent struct type.  */

I'd prefer this to be:

    sorry (...);

Blowing up for things we don't understand is going to make it easier to debug things later. But, it's not a huge deal either way.

+ case VAR_DECL:
+ /* Static variables in a class. */
+ DECL_CONTEXT (child) = type;
+ *fields_tail = child;
+ fields_tail = &TREE_CHAIN (child);
+ break;
+ /* Member functions of a class.
+ FIXME: Extend the DW_TAG_subprogram reader to recognize
+ additional attributes for member functions. */
+ DECL_CONTEXT (child) = type;
+ *methods_tail = child;
+ methods_tail = &TREE_CHAIN (child);
+ break;

I think the DECL_CONTEXT logic should be pushed down into the routines that create the FUNCTION_DECLs/VAR_DECLs. When we create a {FUNCTION,VAR}_DECL we want to set up all of its fields, so that the merge routines can check them at that point. Since this is just for C++, maybe we should just drop these bits for now?

+ 	  /* FIXME: Add support for DW_TAG_access_declaration,
+ 	     DW_TAG_inheritance,  DW_TAG_friend, and DW_TAG_variant_part
+ 	     which can appear as children of a class/struct type.  */

I would add these cases, with a sorry or equivalent; let's not silently accept them.

+   /* The type mode isn't encoded in the DWARF spec, either, so just recompute
+      it from scratch.  */

Yup; we should probably encode that as well, as above for alignment.

Mark Mitchell
(650) 331-3385 x713

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]