On Fri, Feb 24, 2017 at 1:57 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Fri, Feb 24, 2017 at 01:40:37PM -0800, Jason Merrill wrote:
>> On Fri, Feb 24, 2017 at 3:58 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>> > + copy = ggc_alloc<dw_loc_descr_node> ();
>> > + memcpy (copy, ref, sizeof (dw_loc_descr_node));
>> > + list->expr = copy;
>> > + while (copy->dw_loc_next != ref_end)
>> > + {
>> > + dw_loc_descr_ref new_copy = ggc_alloc<dw_loc_descr_node> ();
>> > + memcpy (new_copy, copy->dw_loc_next, sizeof (dw_loc_descr_node));
>> > + copy->dw_loc_next = new_copy;
>> > + copy = new_copy;
>> > + }
>>
>> This seems like it could use a copy_loc_descr function, shared with
>> add_loc_descr_to_each.
>
> Ok, will do that.
>
>> > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (loc)))
>> > + <= DWARF2_ADDR_SIZE))
>>
>> Is this condition necessary, given that DWARF 5 supports typed
>> expressions that are not restricted to the size of the generic type?
>
> The extension pushes the variable value as generic type. We could have
> also DW_OP_GNU_variable_value_type that would provide type and push the
> value as typed value, but we don't need it right now and in the DW_OP_GNU*
> range we don't have many spare positions left. In the standard range there
> are more, so perhaps we can propose for DWARF6 both DW_OP_variable_value
> and DW_OP_variable_value_type.
>
>> > @@ -27532,6 +27538,18 @@ prune_unused_types_walk_loc_descr (dw_lo
>> > if (loc->dw_loc_oprnd1.val_class == dw_val_class_die_ref)
>> > prune_unused_types_mark (loc->dw_loc_oprnd1.v.val_die_ref.die,
>> > 1);
>> > break;
>> > + case DW_OP_GNU_variable_value:
>> > + if (loc->dw_loc_oprnd1.val_class == dw_val_class_decl_ref)
>> > + {
>> > + dw_die_ref ref
>> > + = lookup_decl_die (loc->dw_loc_oprnd1.v.val_decl_ref);
>> > + if (ref == NULL)
>> > + break;
>> > + loc->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
>> > + loc->dw_loc_oprnd1.v.val_die_ref.die = ref;
>> > + loc->dw_loc_oprnd1.v.val_die_ref.external = 0;
>> > + }
>>
>> Doing this adjustment in prune_unused_types is surprising. Why is it
>> needed here as well as in the other places you do it? What if the
>> user specifies -fno-eliminate-unused-debug-types?
>
> This falls through into code that wants to mark the type of the die,
> so this is just an attempt if it can resolve to something, fallthrough
> into that with the die, otherwise we can't do anything and defer it for
> later.
> With -fno-eliminate-unused-debug-types, this will not be done, and only in
> note_variable_value it will try to look it up. The reason for not
> putting note_variable_value before the type pruning is that there is the
> dups elimination and -fdebug-types-section handling that often copies DIEs
> etc. and so I wanted to populate the hash table with the final DIE tree
> layout after the early dwarf.
> Indeed, such lookup_decl_die calls are in multiple places, and if all fail
> to look up, there is code that either will try to do something from the RTL,
> or create the DIE or drop the expression.
Makes sense. The patch is OK with the above refactoring.
Jason
