Re: [patch 10/10] debug-early merge: compiler proper

[Richard Biener <richard.guenther@gmail.com>]

On Fri, May 8, 2015 at 2:40 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> As seen on TV.

+/* FIRST_TIME is set to TRUE for the first time we are called for a
+   translation unit from finalize_compilation_unit() or false
+   otherwise.  */
+
 static void
-analyze_functions (void)
+analyze_functions (bool first_time)
 {
...
+  if (first_time)
+    {
+      symtab_node *snode;
+      FOR_EACH_SYMBOL (snode)
+       check_global_declaration (snode->decl);
+    }
+

I think it is better to split analyze_functions (why does it have it's own copy
of unreachable node removal?) into analysis and unused-symbol removal
and have the
check_global_declaration call be in finalize_compilation_unit directly.  Honza?

@@ -1113,6 +1131,19 @@ analyze_functions (void)
        {
          if (symtab->dump_file)
            fprintf (symtab->dump_file, " %s", node->name ());
+
+         /* See if the debugger can use anything before the DECL
+            passes away.  Perhaps it can notice a DECL that is now a
+            constant and can tag the early DIE with an appropriate
+            attribute.
+
+            Otherwise, this is the last chance the debug_hooks have
+            at looking at optimized away DECLs, since
+            late_global_decl will subsequently be called from the
+            contents of the now pruned symbol table.  */
+         if (!decl_function_context (node->decl))
+           (*debug_hooks->late_global_decl) (node->decl);
+
          node->remove ();

so this applies to VAR_DECLs only - shouldn't this be in the
varpool_node::remove function then?  You can even register/deregister
a hook for this in finalize_compilation_unit.  That would IMHO be better.

All debug stuff apart from dwarf2out.c changes (I assume Jason reviews
them) are ok.

diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index 02012d5..15b6dd2 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -4718,7 +4718,8 @@ write_roots (pair_p variables, bool emit_pch)
    this funcion will have to be adjusted to be more like
    output_mangled_typename.  */

-static void
+/* ?? Why are we keeping this?  Is this actually used anywhere?  */
+static void ATTRIBUTE_UNUSED
 output_typename (outf_p of, const_type_p t)
 {
   switch (t->kind)

Just remove the function.

The langhooks changes are ok.

diff --git a/gcc/passes.c b/gcc/passes.c
index 04ff042..4dee8ad 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -293,6 +293,28 @@ rest_of_decl_compilation (tree decl,
   else if (TREE_CODE (decl) == VAR_DECL && !DECL_EXTERNAL (decl)
           && TREE_STATIC (decl))
     varpool_node::get_create (decl);
+
+  /* Generate early debug for global variables.  Any local variables will
+     be handled by either handling reachable functions from
+     finalize_compilation_unit (and by consequence, locally scoped
+     symbols), or by rest_of_type_compilation below.
+
+     Also, pick up function prototypes, which will be mostly ignored
+     by the different early_global_decl() hooks, but will at least be
+     used by Go's hijack of the debug_hooks to implement
+     -fdump-go-spec.  */
+  if (!flag_wpa
+      && !in_lto_p

Just check !in_lto_p, !flag_wpa is redundant.

+      && !decl_function_context (decl)
+      && !current_function_decl

Why that?  !decl_function_context should catch relevant cases?

+      && !decl_type_context (decl))
+    (*debug_hooks->early_global_decl) (decl);

I'll note that nested functions and class methods are not getting
early_global_decl()ed here.  I suppose their containing function/type
is supposed to generate early dwarf by means of dwarf2out walking
over children.

timevar changes are ok.

the toplev changes are ok.

diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index ad1bb23..2a9f417 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1334,6 +1334,9 @@ struct GTY(()) tree_block {
   tree abstract_origin;
   tree fragment_origin;
   tree fragment_chain;
+
+  /* Pointer to the DWARF lexical block.  */
+  struct die_struct *die;
 };

 struct GTY(()) tree_type_common {

Ick - do we need this?  dwarf2out.c has a hashtable to map blocks to
DIEs (which you don't remove in turn).

Jason - did you intentionally not yet "approve" the dwarf2out.c
changes (like, are you expecting somebody else
to do a review)?

Thanks,
Richard.