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:

-     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.

+      /* 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.

-     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.

+             /* 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?

+      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?

+  /* 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?

-  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.

+  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?

+      /* Variabled-lengthed types may be incomplete even if
+        TREE_ASM_WRITTEN.

"variable-length", I think.

Jason

Reply via email to