Message ID | 201203022129.30464.ebotcazou@adacore.com |
---|---|
State | New |
Headers | show |
On 03/02/2012 03:29 PM, Eric Botcazou wrote: I notice that D.7 seems to suggest that if the nested function is not inlinable and shared between all instances of the containing function that we put a normal (non-abstract/concrete) instance of the nested function inside the abstract function for the containing function. But I agree that it's cleaner your way: put an abstract instance there instead so that the abstract instance of the containing function is all abstract. > + /* Emit an abstract instance of nested functions within an abstract instance > + of their parent. */ > + int declaration = ((decl != current_function_decl > + && !(DECL_INITIAL (decl) != NULL_TREE > + && DECL_ABSTRACT (decl) > + && current_function_decl > + && DECL_ABSTRACT (current_function_decl))) > || class_or_namespace_scope_p (context_die)); Hmm, why isn't current_function_decl == decl when we're trying to emit the abstract instance of a nested function? > + /* Do not emit concrete instances of abstracted nested functions without > + actual instances. */ > + else if (TREE_CODE (decl_or_origin) == FUNCTION_DECL > + && die > + && get_AT (die, DW_AT_inline)) > + ; Should "without actual instances" be something like "within concrete instances of containing functions"? > - if (origin && origin->die_parent) > + if (origin > + && origin->die_parent > + /* Skip an abtract parent instance. */ > + && !(origin->die_parent->die_tag == DW_TAG_subprogram > + && get_AT (origin->die_parent, DW_AT_inline))) > add_child_die (origin->die_parent, die); What if the immediate parent is a DW_TAG_lexical_block or some other thing nested inside an abstract subprogram? > and you suggested to iterate over DECL_CONTEXT instead of die_parent to find an > appropriate parent in order to attach the DIE on the limbo list to. In my earlier comments I seem to have been wrong about the behavior of gen_subprogram_die; now I see that if there is an abstract instance the concrete out-of-line instance is not associated with the decl number. So I guess your earlier limbo handling code was fine apart from the lexical_block issue above. Jason
> I notice that D.7 seems to suggest that if the nested function is not > inlinable and shared between all instances of the containing function > that we put a normal (non-abstract/concrete) instance of the nested > function inside the abstract function for the containing function. But > I agree that it's cleaner your way: put an abstract instance there > instead so that the abstract instance of the containing function is all > abstract. Hum, yes, D.7 even tends to show that what GCC currently generates is as expected. The problem arises when the containing function is itself inlined into the nested function: in this case, we have the cycle in the debug info: |-> AI (containing) -> OL (nested) -> CI (containing) --| | | ------------------DW_AT_abstract_origin ------------------ that is problematic for GDB, and Joel said at the time that there was no easy way to break the recursion in GDB. AI: Abstract Instance OL: Out-of-Line CI: Concrete Instance But if you think the patch is nevertheless reasonable, then let's go ahead. > > + /* Emit an abstract instance of nested functions within an abstract > > instance + of their parent. */ > > + int declaration = ((decl != current_function_decl > > + && !(DECL_INITIAL (decl) != NULL_TREE > > + && DECL_ABSTRACT (decl) > > + && current_function_decl > > + && DECL_ABSTRACT (current_function_decl))) > > > > || class_or_namespace_scope_p (context_die)); > > Hmm, why isn't current_function_decl == decl when we're trying to emit > the abstract instance of a nested function? Because it is emitted when the first instance of the parent function is seen, and in this case current_function_decl == parent_decl. > > + /* Do not emit concrete instances of abstracted nested functions > > without + actual instances. */ > > + else if (TREE_CODE (decl_or_origin) == FUNCTION_DECL > > + && die > > + && get_AT (die, DW_AT_inline)) > > + ; > > Should "without actual instances" be something like "within concrete > instances of containing functions"? Yes, that's probably clearer. > > - if (origin && origin->die_parent) > > + if (origin > > + && origin->die_parent > > + /* Skip an abtract parent instance. */ > > + && !(origin->die_parent->die_tag == DW_TAG_subprogram > > + && get_AT (origin->die_parent, DW_AT_inline))) > > add_child_die (origin->die_parent, die); > > What if the immediate parent is a DW_TAG_lexical_block or some other > thing nested inside an abstract subprogram? Indeed, there was a loop in the last revised version of the original thread: http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01286.html Not clear why I dropped it... > In my earlier comments I seem to have been wrong about the behavior of > gen_subprogram_die; now I see that if there is an abstract instance the > concrete out-of-line instance is not associated with the decl number. > So I guess your earlier limbo handling code was fine apart from the > lexical_block issue above. So the above last revised version is OK with you, modulo the comment in process_scope_var?
On 05/14/2012 11:54 AM, Eric Botcazou wrote: >> Hmm, why isn't current_function_decl == decl when we're trying to emit >> the abstract instance of a nested function? > > Because it is emitted when the first instance of the parent function is seen, > and in this case current_function_decl == parent_decl. Our normal procedure is to generate a declaration when we see a function in its enclosing context, and then fix it up later when we see the definition. Why not handle this similarly? Jason
On 05/14/2012 12:49 PM, Jason Merrill wrote: > On 05/14/2012 11:54 AM, Eric Botcazou wrote: >>> Hmm, why isn't current_function_decl == decl when we're trying to emit >>> the abstract instance of a nested function? >> >> Because it is emitted when the first instance of the parent function >> is seen, >> and in this case current_function_decl == parent_decl. > > Our normal procedure is to generate a declaration when we see a function > in its enclosing context, and then fix it up later when we see the > definition. Why not handle this similarly? I suppose the way we handle nested functions, we generate debug info for the nested function before that for the enclosing function, but then we should attach the (abstract) nested function to the enclosing function in process_scope_var. Jason
> Our normal procedure is to generate a declaration when we see a function > in its enclosing context, and then fix it up later when we see the > definition. Why not handle this similarly? Because we want to generate an abstract instance of the nested function within the abstract instance of the parent function. If we wait for the definition of the nested function, and it's out-of-line, we attach the out-of-line instance to the abstract parent, which is the source of the problem.
On 05/14/2012 04:17 PM, Eric Botcazou wrote: >> Our normal procedure is to generate a declaration when we see a function >> in its enclosing context, and then fix it up later when we see the >> definition. Why not handle this similarly? > > Because we want to generate an abstract instance of the nested function within > the abstract instance of the parent function. If we wait for the definition of > the nested function, and it's out-of-line, we attach the out-of-line instance > to the abstract parent, which is the source of the problem. Right, and that's why we want your change to split the nested function into abstract and concrete instances. But then it should be fine to attach the abstract instance to the abstract parent normally, I would think. Jason
Index: dwarf2out.c =================================================================== --- dwarf2out.c (revision 184668) +++ dwarf2out.c (working copy) @@ -17173,7 +17173,13 @@ gen_subprogram_die (tree decl, dw_die_re dw_die_ref subr_die; tree outer_scope; dw_die_ref old_die = lookup_decl_die (decl); - int declaration = (current_function_decl != decl + /* Emit an abstract instance of nested functions within an abstract instance + of their parent. */ + int declaration = ((decl != current_function_decl + && !(DECL_INITIAL (decl) != NULL_TREE + && DECL_ABSTRACT (decl) + && current_function_decl + && DECL_ABSTRACT (current_function_decl))) || class_or_namespace_scope_p (context_die)); premark_used_types (); @@ -18198,6 +18204,8 @@ gen_inlined_subroutine_die (tree stmt, d gcc_assert (! BLOCK_ABSTRACT (stmt)); decl = block_ultimate_origin (stmt); + if (DECL_IGNORED_P (decl)) + return; /* Emit info for the abstract instance first, if we haven't yet. We must emit this even if the block is abstract, otherwise when we @@ -19158,8 +19166,25 @@ gen_block_die (tree stmt, dw_die_ref con decls_for_scope (stmt, context_die, depth); } +/* Return true if an abstract instance of function DECL can be generated in + the debug information. */ + +static bool +function_possibly_abstracted_p (tree decl) +{ + while (decl) + { + if (cgraph_function_possibly_inlined_p (decl)) + return true; + decl = decl_function_context (decl); + } + + return false; +} + /* Process variable DECL (or variable with origin ORIGIN) within block STMT and add it to CONTEXT_DIE. */ + static void process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die) { @@ -19177,8 +19202,15 @@ process_scope_var (tree stmt, tree decl, if (die != NULL && die->die_parent == NULL) add_child_die (context_die, die); else if (TREE_CODE (decl_or_origin) == IMPORTED_DECL) - dwarf2out_imported_module_or_decl_1 (decl_or_origin, DECL_NAME (decl_or_origin), + dwarf2out_imported_module_or_decl_1 (decl_or_origin, + DECL_NAME (decl_or_origin), stmt, context_die); + /* Do not emit concrete instances of abstracted nested functions without + actual instances. */ + else if (TREE_CODE (decl_or_origin) == FUNCTION_DECL + && die + && get_AT (die, DW_AT_inline)) + ; else gen_decl_die (decl, origin, context_die); } @@ -19525,11 +19557,11 @@ gen_decl_die (tree decl, tree origin, dw ? DECL_ORIGIN (origin) : DECL_ABSTRACT_ORIGIN (decl)); - /* If we're emitting an out-of-line copy of an inline function, + /* If we're emitting an out-of-line copy of an abstracted function, emit info for the abstract instance and set up to refer to it. */ - else if (cgraph_function_possibly_inlined_p (decl) - && ! DECL_ABSTRACT (decl) - && ! class_or_namespace_scope_p (context_die) + else if (!DECL_ABSTRACT (decl) + && function_possibly_abstracted_p (decl) + && !class_or_namespace_scope_p (context_die) /* dwarf2out_abstract_function won't emit a die if this is just a declaration. We must avoid setting DECL_ABSTRACT_ORIGIN in that case, because that works only if we have a die. */ @@ -22526,7 +22558,11 @@ dwarf2out_finish (const char *filename) { dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin); - if (origin && origin->die_parent) + if (origin + && origin->die_parent + /* Skip an abtract parent instance. */ + && !(origin->die_parent->die_tag == DW_TAG_subprogram + && get_AT (origin->die_parent, DW_AT_inline))) add_child_die (origin->die_parent, die); else if (is_cu_die (die)) ; @@ -22545,16 +22581,22 @@ dwarf2out_finish (const char *filename) inlined and optimized out. In that case we are lost and assign the empty child. This should not be big issue as the function is likely unreachable too. */ - tree context = NULL_TREE; - - gcc_assert (node->created_for); + tree context = node->created_for; + gcc_assert (context); - if (DECL_P (node->created_for)) - context = DECL_CONTEXT (node->created_for); - else if (TYPE_P (node->created_for)) - context = TYPE_CONTEXT (node->created_for); + /* Find the first non-abstract parent instance. */ + do { + if (DECL_P (context)) + context = DECL_CONTEXT (context); + else if (TYPE_P (context)) + context = TYPE_CONTEXT (context); + else + context = NULL_TREE; + origin = get_context_die (context); + } while (origin + && origin->die_tag == DW_TAG_subprogram + && get_AT (origin, DW_AT_inline)); - origin = get_context_die (context); add_child_die (origin, die); } }