On Fri, Mar 23, 2018 at 10:58:15AM +0100, Richard Biener wrote:
> On Thu, 22 Mar 2018, Jakub Jelinek wrote:
> 
> > On Thu, Mar 22, 2018 at 03:16:22PM +0100, Richard Biener wrote:
> > > the upper bound decl is global as well).  Jakub, do you remember
> > > a reason for having any constraints here?  I've reworked the
> > 
> > I don't, but it has been added in the
> > https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html
> > patch.  It matches what we were using before:
> > -         if (loc == NULL
> > -             && early_dwarf
> > -             && current_function_decl
> > -             && DECL_CONTEXT (rszdecl) == current_function_decl)
> > when emitting the DW_OP_call4 stuff.
> > 
> > But more importantly, it seems that resolve_variable_value_in_expr
> > will not change anything if
> >       tree decl = loc->dw_loc_oprnd1.v.val_decl_ref;
> >       if (DECL_CONTEXT (decl) != current_function_decl)
> >         continue;
> > Which means resolve_variable_values would never process those
> > DW_OP_GNU_variable_value you've added,
> 
> Ah, ok.  I guess this check can be simply removed.

Actually it should be kept.
resolve_variable_values works on expressions that were added to the hash
table by the note_variable_value_in_expr function, and there a hash table
entry for each containing function and we want to process it only when
doing resolve_variable_values for the particular function e.g. if some
expression happens to be referenced from multiple hash tables.

> > and I believe later on
> > note_variable_value_in_expr will just ICE on it:
> >     if (loc->dw_loc_opc == DW_OP_GNU_variable_value
> >         && loc->dw_loc_oprnd1.val_class == dw_val_class_decl_ref)
> > ...
> >         if (! ref && (flag_generate_lto || flag_generate_offload))
> >           {
> >             /* ???  This is somewhat a hack because we do not create DIEs
> >                for variables not in BLOCK trees early but when generating
> >                early LTO output we need the dw_val_class_decl_ref to be
> >                fully resolved.  For fat LTO objects we'd also like to
> >                undo this after LTO dwarf output.  */
> >             gcc_assert (DECL_CONTEXT (decl));
> > Because DECL_CONTEXT (decl) is NULL, right?
> 
> It's TRANSLATION_UNIT_DECL for the Ada case (we do have FEs where
> decls may have NULL context which then means file-scope).

Anyway, the above code in note_variable_value_in_expr for -flto will ensure
nothing is ever added into variable_value_hash and thus resolve_variable_values
isn't reall invoked in that case.

> Yeah, I guess the whole ! ref case should go and instead drop the
> location expression.  Is there some convenient "NULL" OP we can

There is none, except newly DW_OP_GNU_variable_value referencing a DIE
(say DW_TAG_dwarf_procedure with no DW_AT_location/DW_AT_const_value)
- that is something that never has a usable value, so it is always optimized
out.  That said, resolve_addr_in_expr should handle the removal of
expressions containing DW_OP_GNU_variable_value with dw_val_class_decl_ref
if lookup_decl_die fails.

> I suppose the 2nd hunk is ok.  For reference that was
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c     (revision 258684)
> +++ gcc/dwarf2out.c     (working copy)
> @@ -19878,6 +19878,7 @@ rtl_for_decl_location (tree decl)
>       in the current CU, resolve_addr will remove the expression 
> referencing
>       it.  */
>    if (rtl == NULL_RTX
> +      && !(early_dwarf && (flag_generate_lto || flag_generate_offload))
>        && VAR_P (decl)
>        && !DECL_EXTERNAL (decl)
>        && TREE_STATIC (decl)

Sure, this is obviously ok.

        Jakub

Reply via email to