On 10/28/14 10:28, Jason Merrill wrote:
My apologies for the long delay. I was on PTO.
> On 10/27/2014 08:00 PM, Aldy Hernandez wrote:
2. Changes to gen_variable_die() to handle multiple passes (early/late
dwarf generation).
A lot of this is complicated by the fact that old_die's are cached and
keyed by `tree', but an abstract instance and an inline instance share
trees, while dwarf2out_abstract_function() sets DECL_ABSTRACT_P behind
the scenes.
The current support (and my changes) maintain this shared and delicate
design. I wonder whether we could simplify a lot of this code by
unsharing these trees, but this may be beyond the scope of this work.
Copying all the trees in a function just for debug generation? No, that
sounds undesirable.
3. I've removed deferred_locations. With multiple dwarf passes we can
do without them.
Yay!
Kind words greatly appreciated. Basically I'm looking for feedback
and positive reinforcement that this is all eventually useful
This all looks very good, just a few nitpicks:
Yay!
- instance tree [that has DW_AT_inline] should not contain any
+ instance tree [has DW_AT_inline] should not contain any
This doesn't seem like an improvement.
Reverted.
+ /* Find and reuse a previously generated DW_TAG_subrange_type if
+ available. */
Let's expand this comment a bit to clarify how this works for
multi-dimensional arrays.
Done.
- abstract instance (origin != NULL), in which case we need a new
+ inline instance (origin != NULL), in which case we need a new DIE
I think "concrete instance" is what you want here.
Done.
+ /* Even if we have locations, we need to recurse through
+ the locals to make sure they also have locations. */
Why? What is adding a location to the function without doing the same
for the locals?
Apparently, nothing. I put a gcc_unreachable() there, and in all my
tests, it never got triggered, so I've removed it. Thanks.
+ current_function_has_inlines = 0;
+
+ /* The first time through decls_for_scope we will generate the
+ DIEs for the locals. The second time, we fill in the
+ location info. */
+ decls_for_scope (outer_scope, subr_die, 0);
+
/* Emit a DW_TAG_variable DIE for a named return value. */
if (DECL_NAME (DECL_RESULT (decl)))
gen_decl_die (DECL_RESULT (decl), NULL, subr_die);
- current_function_has_inlines = 0;
- decls_for_scope (outer_scope, subr_die, 0);
Why does this need to be reordered?
This may have been fall back from a previous version. I have reverted
the change.
+ /* If the compiler emitted a definition for the DECL declaration
+ and we already emitted a DIE for it, don't emit a second
+ DIE for it again. Allow re-declarations of DECLs that are
+ inside functions, though. */
+ else if (old_die && !declaration && !local_scope_p (context_die))
+ return;
What DECLs in functions need re-declaration?
This was already there. It is pre-existing code that got moved down
after the new caching code.
- if (decl && (DECL_ABSTRACT_P (decl) || declaration || old_die ==
NULL))
+ if (decl && (DECL_ABSTRACT_P (decl) || declaration || old_die == NULL
+ /* If we make it to a specialization, we have already
+ handled the declaration by virtue of early dwarf.
+ If so, make a new assocation if available, so late
+ dwarf can find it. */
+ || (specialization_p && old_die && old_die->dumped_early)))
equate_decl_number_to_die (decl, var_die);
Instead of old_die->dumped_early, I think it would make more sense to
check early_dwarf_dumping; the reason we need to call
equate_decl_number_to_die is because we're early-dumping the definition
and we will need to find it again later.
I've rewritten the above as:
|| (specialization_p && early_dwarf_dumping)))
+ else if (BLOCK_ABSTRACT_ORIGIN (stmt))
{
+ /* If this is an inlined instance, create a new lexical die for
+ anything below to attach DW_AT_abstract_origin to. */
+ stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
+ }
What if we early dumped this block?
What do you mean? Would you like me to calls decls_for_scope earlier
for abstract instances, or generate the DW_TAG_lexical_block die earlier
for abstract instances, or what?
+ /* Variabled-lengthed types may be incomplete even if
+ TREE_ASM_WRITTEN.
"variable-length", I think.
Fixed in the changelog and otherwise in the patch.
I have committed the attached patch. We can iterate on the
DW_TAG_lexical_block and DECL re-declaration issues in subsequent followups.
As usual, feel free to scream
(https://www.youtube.com/watch?v=HLI4EuDckgM) if in violent disagreement.
Aldy
commit 7d0ab897d086ac8648928b1236dc88697c96d037
Author: Aldy Hernandez <al...@redhat.com>
Date: Fri Dec 5 14:54:41 2014 -0800
* dwarf2out.c (check_die_inline): Revert previous comment change.
(add_subscript_info): Comment better.
(gen_subprogram_die): Do not try to generate local locations if we
already have locations as a whole.
Remove recurse_through_locals label.
Revert reordering of decls_for_scope call.
(gen_variable_die): Check early_dwarf_dumping when testing for
specialization_p instead of checking old_die->dumped_early.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 9cb4ace..3528083 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5558,7 +5558,7 @@ static void
check_die_inline (dw_die_ref die)
{
/* A debugging information entry that is a member of an abstract
- instance tree [has DW_AT_inline] should not contain any
+ instance tree [that has DW_AT_inline] should not contain any
attributes which describe aspects of the subroutine which vary
between distinct inlined expansions or distinct out-of-line
expansions. */
@@ -16637,7 +16637,16 @@ add_subscript_info (dw_die_ref type_die, tree type,
bool collapse_p)
here. */
/* Find and reuse a previously generated DW_TAG_subrange_type if
- available. */
+ available.
+
+ For multi-dimensional arrays, as we iterate through the
+ various dimensions in the enclosing for loop above, we also
+ iterate through the DIE children and pick at each
+ DW_TAG_subrange_type previously generated (if available).
+ Each child DW_TAG_subrange_type DIE describes the range of
+ the current dimension. At this point we should have as many
+ DW_TAG_subrange_type's as we have dimensions in the
+ array. */
dw_die_ref subrange_die = NULL;
if (child)
while (1)
@@ -17327,7 +17336,7 @@ gen_array_type_die (tree type, dw_die_ref context_die)
bool collapse_nested_arrays = !is_ada ();
- /* For variable-lengthed arrays that have been previously generated,
+ /* For variable-length arrays that have been previously generated,
just fill in the possibly missing subscript info. */
if (TREE_ASM_WRITTEN (type)
&& TREE_CODE (type) == ARRAY_TYPE
@@ -17818,8 +17827,8 @@ gen_formal_parameter_die (tree node, tree origin, bool
emit_name_p,
}
/* If we have a previously generated DIE, use it, unless this is an
- inline instance (origin != NULL), in which case we need a new DIE
- with a corresponding DW_AT_abstract_origin. */
+ concrete instance (origin != NULL), in which case we need a new
+ DIE with a corresponding DW_AT_abstract_origin. */
bool reusing_die;
if (parm_die && origin == NULL)
reusing_die = true;
@@ -18418,14 +18427,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
something we have already output. */
if (get_AT (old_die, DW_AT_low_pc)
|| get_AT (old_die, DW_AT_ranges))
- {
- /* Even if we have locations, we need to recurse through
- the locals to make sure they also have locations. */
- if (dumped_early)
- goto recurse_through_locals;
-
- return;
- }
+ return;
/* If we have no location information, this must be a
partially generated DIE from early dwarf generation.
@@ -18835,7 +18837,6 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
/* Add the calling convention attribute if requested. */
add_calling_convention_attribute (subr_die, decl);
- recurse_through_locals:
/* Output Dwarf info for all of the stuff within the body of the function
(if it has one - it may be just a declaration).
@@ -18859,6 +18860,10 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
int call_site_note_count = 0;
int tail_call_site_note_count = 0;
+ /* Emit a DW_TAG_variable DIE for a named return value. */
+ if (DECL_NAME (DECL_RESULT (decl)))
+ gen_decl_die (DECL_RESULT (decl), NULL, subr_die);
+
current_function_has_inlines = 0;
/* The first time through decls_for_scope we will generate the
@@ -18866,10 +18871,6 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
location info. */
decls_for_scope (outer_scope, subr_die, 0);
- /* Emit a DW_TAG_variable DIE for a named return value. */
- if (DECL_NAME (DECL_RESULT (decl)))
- gen_decl_die (DECL_RESULT (decl), NULL, subr_die);
-
if (call_arg_locations && !dwarf_strict)
{
struct call_arg_loc_node *ca_loc;
@@ -19312,7 +19313,7 @@ gen_variable_die (tree decl, tree origin, dw_die_ref
context_die)
handled the declaration by virtue of early dwarf.
If so, make a new assocation if available, so late
dwarf can find it. */
- || (specialization_p && old_die && old_die->dumped_early)))
+ || (specialization_p && early_dwarf_dumping)))
equate_decl_number_to_die (decl, var_die);
gen_variable_die_location:
