On Sat, 17 Nov 2018, Jakub Jelinek wrote:
> On Sat, Nov 17, 2018 at 05:51:05PM +0100, Richard Biener wrote:
> > On November 17, 2018 4:14:58 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com>
> > wrote:
> > >On Sat, Nov 17, 2018 at 08:31:32AM +0100, Richard Biener wrote:
> > >> On November 16, 2018 10:10:00 PM GMT+01:00, Jakub Jelinek
> > >> <ja...@redhat.com> wrote: Can you add a comment why doing it
> > >differently
> > >> for this case is necessary or do it the same way in all cases?
> > >
> > >Do you mean in omp-expand.c or dwarf2out.c? I believe I'm doing it the
> > >same
> >
> > I meant in dwarf2out.c. IIRC we have multiple calls into dwarf2out_decl
> > and you adjust only one?
>
> We don't need to change the case where decl isn't a FUNCTION_DECL,
> and must not change the case where we call dwarf2out_decl (decl), that would
> lead to infinite recursion.
>
> The only case might be replace
> current_function_decl = origin;
> dwarf2out_decl (origin);
> with the dwarf2out_early_global_decl (origin); call, not really sure if that
> is needed though. Do we ever have functions where
> decl_function_context (decl) != decl_function_context (DECL_ABSTRACT_ORIGIN
> (decl))
> ?
>
> The other possibility is to move this decl_function_context handling code
> from dwarf2out_early_global_decl to dwarf2out_decl, guarded with
> if (early_dwarf
> && decl_function_context (decl)))
> ...
>
> Something like (completely untested):
>
> --- gcc/dwarf2out.c.jj 2018-11-16 17:33:42.899215778 +0100
> +++ gcc/dwarf2out.c 2018-11-17 20:11:26.847301806 +0100
> @@ -26390,22 +26390,6 @@ dwarf2out_early_global_decl (tree decl)
> {
> tree save_fndecl = current_function_decl;
>
> - /* For nested functions, make sure we have DIEs for the parents first
> - so that all nested DIEs are generated at the proper scope in the
> - first shot. */
> - tree context = decl_function_context (decl);
> - if (context != NULL)
> - {
> - dw_die_ref context_die = lookup_decl_die (context);
> - current_function_decl = context;
> -
> - /* Avoid emitting DIEs multiple times, but still process CONTEXT
> - enough so that it lands in its own context. This avoids type
> - pruning issues later on. */
> - if (context_die == NULL || is_declaration_die (context_die))
> - dwarf2out_decl (context);
> - }
> -
> /* Emit an abstract origin of a function first. This happens
> with C++ constructor clones for example and makes
> dwarf2out_abstract_function happy which requires the early
> @@ -26716,11 +26700,31 @@ dwarf2out_decl (tree decl)
> we're a method, it will be ignored, since we already have a DIE.
> Avoid doing this late though since clones of class methods may
> otherwise end up in limbo and create type DIEs late. */
> - if (early_dwarf
> - && decl_function_context (decl)
> - /* But if we're in terse mode, we don't care about scope. */
> - && debug_info_level > DINFO_LEVEL_TERSE)
> - context_die = NULL;
> + if (early_dwarf)
> + {
> + /* For nested functions, make sure we have DIEs for the parents first
> + so that all nested DIEs are generated at the proper scope in the
> + first shot. */
> + tree context = decl_function_context (decl);
> + if (context != NULL)
> + {
> + dw_die_ref ctx_die = lookup_decl_die (context);
> + tree save_fndecl = current_function_decl;
> + current_function_decl = context;
> +
> + /* Avoid emitting DIEs multiple times, but still process CONTEXT
> + enough so that it lands in its own context. This avoids type
> + pruning issues later on. */
> + if (ctx_die == NULL || is_declaration_die (ctx_die))
> + dwarf2out_decl (context);
> +
> + current_function_decl = save_fndecl;
> +
> + /* But if we're in terse mode, we don't care about scope. */
> + if (debug_info_level > DINFO_LEVEL_TERSE)
> + context_die = NULL;
> + }
> + }
Ugh, I'd like to keep the ordering "hacks" in dwarf2out_early_global_decl.
Reviewing the original code now the only case we need the recursion
is that of your original change (though the recursion shouldn't
be necessary for the is_declaration_die case).
Thus the original dwarf2out.c hunk is OK.
Thanks and sorry for the confusion coming from reviewing patches
from a mobile phone w/o SVN access...
Richard.
> break;
>
> case VAR_DECL:
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
