> On Fri, May 8, 2015 at 2:40 AM, Aldy Hernandez <al...@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?
It is trying to avoid analyzing functions that are never used. Analyzing
include gimplification and other stuff that is not for free.
remove_unreachable_nodes works only on fully built cgraph.
Main reason why analyze_functions is intended to be called multiple times was
--combine support. After each end of C source file you called
symbol_table::finalize_compilation_unit which did unreachable code removal and
saved some memory.
I am not sure symbol table is useful for debug info this way: we insert only
function definitions into symbol table and external declarations are inserted
lazilly. That is why we still have the (convoluted) check_global_declarations
that walks the vectors provided by frontends.
>
> @@ -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.
There is varpool_node::remove_initializer that is intended to throw away the
constructor. I suppose late_global_decl can be called from there?
> @@ -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).
Note that types also have tree_type_symtab that in turn contains die pointer.
Honza
>
> Jason - did you intentionally not yet "approve" the dwarf2out.c
> changes (like, are you expecting somebody else
> to do a review)?
>
> Thanks,
> Richard.
