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.

        Jakub

Reply via email to