Hi,
IPA comdats performs a dataflow identifying section where every symbol is used.
It sanity checks that everything is reachable. This sanity check shows latent
issue with unreachable function removal.
symbol_table::remove_unreachable_nodes has parameter before_inlining_p that
says whether extern inline and virtual functions should be eliminated if they
are not inlined.  This parameter is correctly used only in call within inliner
itself and in cgraphunit.  All other cleanups happens with before_inlining_p
true that may leave some unreachable inlines at ipa-comdats time.

I fixed this by adding explicit state of symbol table for IPA passes run after
inliner.

Another issue found by Trevor is that ipa-pure-const may render function 
unreachable
in case a static cdtor is found to be pure/const. Fixed thus.

I also updated ipa.c to be more agressive on removing functions that may be 
inlined
at -O0.  This should improve compile times since the functions do not need to 
bubble
down in the queue.

In fact there is same issue in reachability computed at callgraph construction 
time
as well as within C++ frontend.  I will send separate patches for this: it 
seems that
those may account quite noticeable percentage of memory and compile time.

Bootstrapped/regtested x86_64-linux, comitted.
I am grateful to Trevor for analysis and initial patch.

Honza

        PR ipa/61324
        * testsuite/g++.dg/pr61324.C: New testcase by Trevor Saunders.
        * testsuite/g++.dg/tm/pr51411-2.C: Update se the extern function is
        not eliminated early.
        * testsuite/gcc.target/i386/pr57756.c: Turn extern inline into static
        inline.

        * passes.c (execute_todo): Update call of remove_unreachable_nodes.
        * ipa-chkp.c (chkp_produce_thunks): Use TODO_remove_functions.
        * cgraphunit.c (symbol_table::process_new_functions): Add
        IPA_SSA_AFTER_INLINING.
        (ipa_passes): Update call of remove_unreachable_nodes.
        (symbol_table::compile): Remove call of remove_unreachable_nodes.
        * ipa-inline.c (inline_small_functions): Do not ICE with
        -flto-partition=none
        (ipa_inline): Update symtab->state; fix formatting
        update call of remove_unreachable_nodes.
        * cgraphclones.c (symbol_table::materialize_all_clones): Likewise.
        * cgraph.h (enum symtab_state): Add IPA_SSA_AFTER_INLINING.
        (remove_unreachable_nodes): Update.
        * ipa.c (process_references): Keep external references only
        when optimizing.
        (walk_polymorphic_call_targets): Keep possible polymorphic call
        target only when devirtualizing.
        (symbol_table::remove_unreachable_nodes): Remove BEFORE_INLINING_P
        parameter.
        (ipa_single_use): Update comment.
        * ipa-pure-const.c (cdtor_p): New function.
        (propagate_pure_const): Track if some cdtor was turned pure/const.
        (execute): Return TODO_remove_functions if needed.
        * ipa-comdats.c (ipa_comdats): Update comment.
        
        * lto.c (read_cgraph_and_symbols): Update call of
        remove_unreachable_nodes.
        (do_whole_program_analysis): Remove call of
        symtab->remove_unreachable_nodes
Index: testsuite/g++.dg/pr61324.C
===================================================================
--- testsuite/g++.dg/pr61324.C  (revision 0)
+++ testsuite/g++.dg/pr61324.C  (revision 0)
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "-O -fkeep-inline-functions -fno-use-cxa-atexit" }
+void foo ();
+
+struct S
+{
+  ~S ()
+  {
+    foo ();
+  }
+};
+
+S s;
Index: testsuite/g++.dg/tm/pr51411-2.C
===================================================================
--- testsuite/g++.dg/tm/pr51411-2.C     (revision 218610)
+++ testsuite/g++.dg/tm/pr51411-2.C     (working copy)
@@ -26,6 +26,7 @@ public:
     bool compare(const basic_string& __str) const {
         return 0;
     }
+    void key ();
 };
 
 typedef basic_string<char> string;
@@ -35,7 +36,7 @@ inline bool operator<(const basic_string
     return __lhs.compare(__rhs);
 }
 
-extern template class basic_string<char>;
+template class basic_string<char>;
 
 }
 
Index: testsuite/gcc.target/i386/pr57756.c
===================================================================
--- testsuite/gcc.target/i386/pr57756.c (revision 218610)
+++ testsuite/gcc.target/i386/pr57756.c (working copy)
@@ -9,7 +9,7 @@ __inline int callee () /* { dg-error "in
 }
 
 __attribute__((target("sse")))
-__inline int caller ()
+static __inline int caller ()
 {
   return callee(); /* { dg-error "called from here" }  */
 }
Index: cgraph.h
===================================================================
--- cgraph.h    (revision 218610)
+++ cgraph.h    (working copy)
@@ -1801,12 +1801,15 @@ enum symtab_state
   PARSING,
   /* Callgraph is being constructed.  It is safe to add new functions.  */
   CONSTRUCTION,
-  /* Callgraph is being at LTO time.  */
+  /* Callgraph is being streamed-in at LTO time.  */
   LTO_STREAMING,
-  /* Callgraph is built and IPA passes are being run.  */
+  /* Callgraph is built and early IPA passes are being run.  */
   IPA,
   /* Callgraph is built and all functions are transformed to SSA form.  */
   IPA_SSA,
+  /* All inline decisions are done; it is now possible to remove extern inline
+     functions and virtual call targets.  */
+  IPA_SSA_AFTER_INLINING,
   /* Functions are now ordered and being passed to RTL expanders.  */
   EXPANSION,
   /* All cgraph expansion is done.  */
@@ -1876,7 +1879,7 @@ public:
   }
 
   /* Perform reachability analysis and reclaim all unreachable nodes.  */
-  bool remove_unreachable_nodes (bool before_inlining_p, FILE *file);
+  bool remove_unreachable_nodes (FILE *file);
 
   /* Optimization of function bodies might've rendered some variables as
      unnecessary so we want to avoid these from being compiled.  Re-do
Index: ipa.c
===================================================================
--- ipa.c       (revision 218610)
+++ ipa.c       (working copy)
@@ -123,21 +123,33 @@ process_references (symtab_node *snode,
   for (i = 0; snode->iterate_reference (i, ref); i++)
     {
       symtab_node *node = ref->referred;
+      symtab_node *body = node->ultimate_alias_target ();
 
       if (node->definition && !node->in_other_partition
          && ((!DECL_EXTERNAL (node->decl) || node->alias)
              || (((before_inlining_p
-                   && (symtab->state < IPA_SSA
-                       || !lookup_attribute ("always_inline",
-                                             DECL_ATTRIBUTES (node->decl)))))
-                 /* We use variable constructors during late complation for
+                   && (TREE_CODE (node->decl) != FUNCTION_DECL
+                       || opt_for_fn (body->decl, optimize)
+                       || (symtab->state < IPA_SSA
+                           && lookup_attribute
+                                ("always_inline",
+                                 DECL_ATTRIBUTES (body->decl))))))
+                 /* We use variable constructors during late compilation for
                     constant folding.  Keep references alive so partitioning
                     knows about potential references.  */
                  || (TREE_CODE (node->decl) == VAR_DECL
                      && flag_wpa
                      && ctor_for_folding (node->decl)
                         != error_mark_node))))
-       reachable->add (node);
+       {
+         /* Be sure that we will not optimize out alias target
+            body.  */
+         if (DECL_EXTERNAL (node->decl)
+             && node->alias
+             && before_inlining_p)
+           reachable->add (body);
+         reachable->add (node);
+       }
       enqueue_node (node, first, reachable);
     }
 }
@@ -178,15 +190,23 @@ walk_polymorphic_call_targets (hash_set<
                    (method_class_type (TREE_TYPE (n->decl))))
            continue;
 
+          symtab_node *body = n->function_symbol ();
+
          /* Prior inlining, keep alive bodies of possible targets for
             devirtualization.  */
           if (n->definition
               && (before_inlining_p
-                  && (symtab->state < IPA_SSA
-                      || !lookup_attribute ("always_inline",
-                                            DECL_ATTRIBUTES (n->decl)))))
-            reachable->add (n);
-
+                  && opt_for_fn (body->decl, optimize)
+                  && opt_for_fn (body->decl, flag_devirtualize)))
+             {
+                /* Be sure that we will not optimize out alias target
+                   body.  */
+                if (DECL_EXTERNAL (n->decl)
+                    && n->alias
+                    && before_inlining_p)
+                  reachable->add (body);
+               reachable->add (n);
+             }
          /* Even after inlining we want to keep the possible targets in the
             boundary, so late passes can still produce direct call even if
             the chance for inlining is lost.  */
@@ -246,8 +266,6 @@ walk_polymorphic_call_targets (hash_set<
      After inlining we release their bodies and turn them into unanalyzed
      nodes even when they are reachable.
 
-     BEFORE_INLINING_P specify whether we are before or after inlining.
-
    - virtual functions are kept in callgraph even if they seem unreachable in
      hope calls to them will be devirtualized. 
 
@@ -293,7 +311,7 @@ walk_polymorphic_call_targets (hash_set<
    we set AUX pointer of processed symbols in the boundary to constant 2.  */
 
 bool
-symbol_table::remove_unreachable_nodes (bool before_inlining_p, FILE *file)
+symbol_table::remove_unreachable_nodes (FILE *file)
 {
   symtab_node *first = (symtab_node *) (void *) 1;
   struct cgraph_node *node, *next;
@@ -302,6 +320,8 @@ symbol_table::remove_unreachable_nodes (
   hash_set<symtab_node *> reachable;
   hash_set<tree> body_needed_for_clonning;
   hash_set<void *> reachable_call_targets;
+  bool before_inlining_p = symtab->state < (!optimize ? IPA_SSA
+                                           : IPA_SSA_AFTER_INLINING);
 
   timevar_push (TV_IPA_UNREACHABLE);
   build_type_inheritance_graph ();
@@ -414,19 +434,25 @@ symbol_table::remove_unreachable_nodes (
                }
              for (e = cnode->callees; e; e = e->next_callee)
                {
+                 symtab_node *body = e->callee->function_symbol ();
                  if (e->callee->definition
                      && !e->callee->in_other_partition
                      && (!e->inline_failed
                          || !DECL_EXTERNAL (e->callee->decl)
                          || e->callee->alias
-                         || before_inlining_p))
+                         || (before_inlining_p
+                             && (opt_for_fn (body->decl, optimize)
+                                 || (symtab->state < IPA_SSA
+                                     && lookup_attribute
+                                         ("always_inline",
+                                          DECL_ATTRIBUTES (body->decl)))))))
                    {
                      /* Be sure that we will not optimize out alias target
                         body.  */
                      if (DECL_EXTERNAL (e->callee->decl)
                          && e->callee->alias
                          && before_inlining_p)
-                       reachable.add (e->callee->function_symbol ());
+                       reachable.add (body);
                      reachable.add (e->callee);
                    }
                  enqueue_node (e->callee, &first, &reachable);
@@ -1219,14 +1245,15 @@ propagate_single_user (varpool_node *vno
            function = BOTTOM;
        }
       else
-        function = meet (function, dyn_cast <varpool_node *> (ref->referring), 
single_user_map);
+       function = meet (function, dyn_cast <varpool_node *> (ref->referring),
+                        single_user_map);
     }
   return function;
 }
 
 /* Pass setting used_by_single_function flag.
-   This flag is set on variable when there is only one function that may 
possibly
-   referr to it.  */
+   This flag is set on variable when there is only one function that may
+   possibly referr to it.  */
 
 static unsigned int
 ipa_single_use (void)
@@ -1304,7 +1331,10 @@ ipa_single_use (void)
       if (var->aux != BOTTOM)
        {
 #ifdef ENABLE_CHECKING
-         if (!single_user_map.get (var))
+         /* Not having the single user known means that the VAR is
+            unreachable.  Either someone forgot to remove unreachable
+            variables or the reachability here is wrong.  */
+
           gcc_assert (single_user_map.get (var));
 #endif
          if (dump_file)
Index: cgraphclones.c
===================================================================
--- cgraphclones.c      (revision 218610)
+++ cgraphclones.c      (working copy)
@@ -1146,7 +1146,7 @@ symbol_table::materialize_all_clones (vo
 #ifdef ENABLE_CHECKING
   cgraph_node::verify_cgraph_nodes ();
 #endif
-  symtab->remove_unreachable_nodes (false, symtab->dump_file);
+  symtab->remove_unreachable_nodes (symtab->dump_file);
 }
 
 #include "gt-cgraphclones.h"
Index: cgraphunit.c
===================================================================
--- cgraphunit.c        (revision 218610)
+++ cgraphunit.c        (working copy)
@@ -327,6 +327,7 @@ symbol_table::process_new_functions (voi
 
        case IPA:
        case IPA_SSA:
+       case IPA_SSA_AFTER_INLINING:
          /* When IPA optimization already started, do all essential
             transformations that has been already performed on the whole
             cgraph but not on this function.  */
@@ -335,7 +336,7 @@ symbol_table::process_new_functions (voi
          if (!node->analyzed)
            node->analyze ();
          push_cfun (DECL_STRUCT_FUNCTION (fndecl));
-         if (state == IPA_SSA
+         if ((state == IPA_SSA || state == IPA_SSA_AFTER_INLINING)
              && !gimple_in_ssa_p (DECL_STRUCT_FUNCTION (fndecl)))
            g->get_passes ()->execute_early_local_passes ();
          else if (inline_summary_vec != NULL)
@@ -507,6 +508,7 @@ cgraph_node::add_new_function (tree fnde
 
       case IPA:
       case IPA_SSA:
+      case IPA_SSA_AFTER_INLINING:
       case EXPANSION:
        /* Bring the function into finalized state and enqueue for later
           analyzing and compilation.  */
@@ -2053,7 +2055,7 @@ ipa_passes (void)
 
   /* This extra symtab_remove_unreachable_nodes pass tends to catch some
      devirtualization and other changes where removal iterate.  */
-  symtab->remove_unreachable_nodes (true, symtab->dump_file);
+  symtab->remove_unreachable_nodes (symtab->dump_file);
 
   /* If pass_all_early_optimizations was not scheduled, the state of
      the cgraph will not be properly updated.  Update it now.  */
@@ -2194,10 +2196,6 @@ symbol_table::compile (void)
       return;
     }
 
-  /* This pass remove bodies of extern inline functions we never inlined.
-     Do this later so other IPA passes see what is really going on.
-     FIXME: This should be run just after inlining by pasmanager.  */
-  remove_unreachable_nodes (false, dump_file);
   global_info_ready = true;
   if (dump_file)
     {
Index: lto/lto.c
===================================================================
--- lto/lto.c   (revision 218610)
+++ lto/lto.c   (working copy)
@@ -3098,7 +3098,7 @@ read_cgraph_and_symbols (unsigned nfiles
   /* Removal of unreachable symbols is needed to make verify_symtab to pass;
      we are still having duplicated comdat groups containing local statics.
      We could also just remove them while merging.  */
-  symtab->remove_unreachable_nodes (true, dump_file);
+  symtab->remove_unreachable_nodes (dump_file);
   ggc_collect ();
   symtab->state = IPA_SSA;
 
@@ -3255,7 +3255,6 @@ do_whole_program_analysis (void)
   symtab->state = IPA_SSA;
 
   execute_ipa_pass_list (g->get_passes ()->all_regular_ipa_passes);
-  symtab->remove_unreachable_nodes (false, dump_file);
 
   if (symtab->dump_file)
     {
Index: ipa-inline.c
===================================================================
--- ipa-inline.c        (revision 218610)
+++ ipa-inline.c        (working copy)
@@ -1731,9 +1731,9 @@ inline_small_functions (void)
                   " to be inlined into %s/%i in %s:%i\n"
                   " Estimated badness is %"PRId64", frequency %.2f.\n",
                   edge->caller->name (), edge->caller->order,
-                  flag_wpa ? "unknown"
+                  edge->call_stmt ? "unknown"
                   : gimple_filename ((const_gimple) edge->call_stmt),
-                  flag_wpa ? -1
+                  edge->call_stmt ? -1
                   : gimple_lineno ((const_gimple) edge->call_stmt),
                   badness.to_int (),
                   edge->frequency / (double)CGRAPH_FREQ_BASE);
@@ -2188,9 +2188,12 @@ ipa_inline (void)
 
   inline_small_functions ();
 
-  /* Do first after-inlining removal.  We want to remove all "stale" extern 
inline
-     functions and virtual functions so we really know what is called once.  */
-  symtab->remove_unreachable_nodes (false, dump_file);
+  gcc_assert (symtab->state == IPA_SSA);
+  symtab->state = IPA_SSA_AFTER_INLINING;
+  /* Do first after-inlining removal.  We want to remove all "stale" extern
+     inline functions and virtual functions so we really know what is called
+     once.  */
+  symtab->remove_unreachable_nodes (dump_file);
   free (order);
 
   /* Inline functions with a property that after inlining into all callers the
@@ -2199,7 +2202,8 @@ ipa_inline (void)
      are met.  */
   if (dump_file)
     fprintf (dump_file,
-            "\nDeciding on functions to be inlined into all callers and 
removing useless speculations:\n");
+            "\nDeciding on functions to be inlined into all callers and "
+            "removing useless speculations:\n");
 
   /* Inlining one function called once has good chance of preventing
      inlining other function into the same callee.  Ideally we should
@@ -2247,10 +2251,11 @@ ipa_inline (void)
              int num_calls = 0;
              node->call_for_symbol_thunks_and_aliases (sum_callers, &num_calls,
                                                      true);
-             while (node->call_for_symbol_thunks_and_aliases 
(inline_to_all_callers,
-                                                            &num_calls, true))
+             while (node->call_for_symbol_thunks_and_aliases
+                      (inline_to_all_callers, &num_calls, true))
                ;
-             remove_functions = true;
+             if (num_calls)
+               remove_functions = true;
            }
        }
     }
Index: ipa-chkp.c
===================================================================
--- ipa-chkp.c  (revision 218610)
+++ ipa-chkp.c  (working copy)
@@ -647,9 +647,7 @@ chkp_produce_thunks (void)
        chkp_function_mark_instrumented (node->decl);
     }
 
-  symtab->remove_unreachable_nodes (true, dump_file);
-
-  return 0;
+  return TODO_remove_functions;
 }
 
 const pass_data pass_data_ipa_chkp_versioning =
Index: ipa-comdats.c
===================================================================
--- ipa-comdats.c       (revision 218610)
+++ ipa-comdats.c       (working copy)
@@ -327,7 +327,14 @@ ipa_comdats (void)
          && !symbol->alias
          && symbol->real_symbol_p ())
        {
-         tree group = *map.get (symbol);
+         tree *val = map.get (symbol);
+
+         /* A NULL here means that SYMBOL is unreachable in the definition
+            of ipa-comdats. Either ipa-comdats is wrong about this or someone
+            forgot to cleanup and remove unreachable functions earlier.  */
+         gcc_assert (val);
+
+         tree group = *val;
 
          if (group == error_mark_node)
            continue;
Index: ipa-pure-const.c
===================================================================
--- ipa-pure-const.c    (revision 218610)
+++ ipa-pure-const.c    (working copy)
@@ -1160,10 +1160,21 @@ self_recursive_p (struct cgraph_node *no
   return false;
 }
 
+/* Return true if N is cdtor that is not const or pure.  In this case we may
+   need to remove unreachable function if it is marked const/pure.  */
+
+static bool
+cdtor_p (cgraph_node *n, void *)
+{
+  if (DECL_STATIC_CONSTRUCTOR (n->decl) || DECL_STATIC_DESTRUCTOR (n->decl))
+    return !TREE_READONLY (n->decl) && !DECL_PURE_P (n->decl);
+  return false;
+}
+
 /* Produce transitive closure over the callgraph and compute pure/const
    attributes.  */
 
-static void
+static bool
 propagate_pure_const (void)
 {
   struct cgraph_node *node;
@@ -1173,6 +1184,7 @@ propagate_pure_const (void)
   int order_pos;
   int i;
   struct ipa_dfs_info * w_info;
+  bool remove_p = false;
 
   order_pos = ipa_reduced_postorder (order, true, false, NULL);
   if (dump_file)
@@ -1447,6 +1459,8 @@ propagate_pure_const (void)
                               this_looping ? "looping " : "",
                               w->name ());
                  }
+               remove_p |= w->call_for_symbol_and_aliases (cdtor_p,
+                                                           NULL, true);
                w->set_const_flag (true, this_looping);
                break;
 
@@ -1459,6 +1473,8 @@ propagate_pure_const (void)
                               this_looping ? "looping " : "",
                               w->name ());
                  }
+               remove_p |= w->call_for_symbol_and_aliases (cdtor_p,
+                                                           NULL, true);
                w->set_pure_flag (true, this_looping);
                break;
 
@@ -1472,6 +1488,7 @@ propagate_pure_const (void)
 
   ipa_free_postorder_info ();
   free (order);
+  return remove_p;
 }
 
 /* Produce transitive closure over the callgraph and compute nothrow
@@ -1581,6 +1598,7 @@ pass_ipa_pure_const::
 execute (function *)
 {
   struct cgraph_node *node;
+  bool remove_p;
 
   symtab->remove_cgraph_insertion_hook (function_insertion_hook_holder);
   symtab->remove_cgraph_duplication_hook (node_duplication_hook_holder);
@@ -1589,14 +1607,14 @@ execute (function *)
   /* Nothrow makes more function to not lead to return and improve
      later analysis.  */
   propagate_nothrow ();
-  propagate_pure_const ();
+  remove_p = propagate_pure_const ();
 
   /* Cleanup. */
   FOR_EACH_FUNCTION (node)
     if (has_function_state (node))
       free (get_function_state (node));
   funct_state_vec.release ();
-  return 0;
+  return remove_p ? TODO_remove_functions : 0;
 }
 
 static bool
Index: passes.c
===================================================================
--- passes.c    (revision 218610)
+++ passes.c    (working copy)
@@ -2003,7 +2003,7 @@ execute_todo (unsigned int flags)
   if (flags & TODO_remove_functions)
     {
       gcc_assert (!cfun);
-      symtab->remove_unreachable_nodes (true, dump_file);
+      symtab->remove_unreachable_nodes (dump_file);
     }
 
   if ((flags & TODO_dump_symtab) && dump_file && !current_function_decl)

Reply via email to