C++ PATCH to set DECL_COMDAT on undefined inlines/templates

[Jason Merrill <jason at redhat dot com>]

Since can_refer_decl_in_current_unit_p doesn't allow references to 
DECL_COMDAT entities that aren't defined in the current unit, by setting 
that flag we can avoid problems with devirtualization introducing 
references to vague linkage functions that haven't been 
synthesized/instantiated because they aren't referenced directly.

The first attached patch sets DECL_COMDAT for inlines and templates.

The second attached patch removes the -fuse-all-virtuals code, which is 
no longer needed for correctness.

Tested x86_64-pc-linux-gnu, applying to trunk.  I'm planning to apply 
the first patch to 4.9.2 if it doesn't cause trouble on trunk.
commit b591032bfe618a612ab26ace8039c64c8eac3fd2
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jul 29 17:28:44 2014 -0400

    	PR lto/53808
    	PR c++/61659
    	* pt.c (push_template_decl_real): Set DECL_COMDAT on templates.
    	(check_explicit_specialization): Clear it on specializations.
    	* decl.c (duplicate_decls, start_decl): Likewise.
    	(grokmethod, grokfndecl): Set DECL_COMDAT on inlines.
    	* method.c (implicitly_declare_fn): Set DECL_COMDAT.  Determine
    	linkage after setting the appropriate flags.
    	* tree.c (decl_linkage): Don't check DECL_COMDAT.
    	* decl2.c (mark_needed): Mark clones.
    	(import_export_decl): Not here.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index aafb917..fd5e2e5 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2197,6 +2197,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
 		      olddecl);
 
 	  SET_DECL_TEMPLATE_SPECIALIZATION (olddecl);
+	  DECL_COMDAT (newdecl) = DECL_DECLARED_INLINE_P (olddecl);
 
 	  /* Don't propagate visibility from the template to the
 	     specialization here.  We'll do that in determine_visibility if
@@ -4683,6 +4684,10 @@ start_decl (const cp_declarator *declarator,
       if (DECL_LANG_SPECIFIC (decl) && DECL_USE_TEMPLATE (decl))
 	{
 	  SET_DECL_TEMPLATE_SPECIALIZATION (decl);
+	  if (TREE_CODE (decl) == FUNCTION_DECL)
+	    DECL_COMDAT (decl) = DECL_DECLARED_INLINE_P (decl);
+	  else
+	    DECL_COMDAT (decl) = false;
 
 	  /* [temp.expl.spec] An explicit specialization of a static data
 	     member of a template is a definition if the declaration
@@ -7663,7 +7668,10 @@ grokfndecl (tree ctype,
 
   /* If the declaration was declared inline, mark it as such.  */
   if (inlinep)
-    DECL_DECLARED_INLINE_P (decl) = 1;
+    {
+      DECL_DECLARED_INLINE_P (decl) = 1;
+      DECL_COMDAT (decl) = 1;
+    }
   if (inlinep & 2)
     DECL_DECLARED_CONSTEXPR_P (decl) = true;
 
@@ -14223,6 +14231,7 @@ grokmethod (cp_decl_specifier_seq *declspecs,
 
   check_template_shadow (fndecl);
 
+  DECL_COMDAT (fndecl) = 1;
   DECL_DECLARED_INLINE_P (fndecl) = 1;
   DECL_NO_INLINE_WARNING_P (fndecl) = 1;
 
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 8fa3145..884be0a 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1930,6 +1930,12 @@ mark_needed (tree decl)
 	 definition.  */
       struct cgraph_node *node = cgraph_node::get_create (decl);
       node->forced_by_abi = true;
+
+      /* #pragma interface and -frepo code can call mark_needed for
+          maybe-in-charge 'tors; mark the clones as well.  */
+      tree clone;
+      FOR_EACH_CLONE (clone, decl)
+	mark_needed (clone);
     }
   else if (TREE_CODE (decl) == VAR_DECL)
     {
@@ -2728,17 +2734,7 @@ import_export_decl (tree decl)
     {
       /* The repository indicates that this entity should be defined
 	 here.  Make sure the back end honors that request.  */
-      if (VAR_P (decl))
-	mark_needed (decl);
-      else if (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl)
-	       || DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl))
-	{
-	  tree clone;
-	  FOR_EACH_CLONE (clone, decl)
-	    mark_needed (clone);
-	}
-      else
-	mark_needed (decl);
+      mark_needed (decl);
       /* Output the definition as an ordinary strong definition.  */
       DECL_EXTERNAL (decl) = 0;
       DECL_INTERFACE_KNOWN (decl) = 1;
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index e5fa0c1..f86a214 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1798,8 +1798,6 @@ implicitly_declare_fn (special_function_kind kind, tree type,
   DECL_ARGUMENTS (fn) = this_parm;
 
   grokclassfn (type, fn, kind == sfk_destructor ? DTOR_FLAG : NO_SPECIAL);
-  set_linkage_according_to_type (type, fn);
-  rest_of_decl_compilation (fn, toplevel_bindings_p (), at_eof);
   DECL_IN_AGGR_P (fn) = 1;
   DECL_ARTIFICIAL (fn) = 1;
   DECL_DEFAULTED_FN (fn) = 1;
@@ -1811,6 +1809,9 @@ implicitly_declare_fn (special_function_kind kind, tree type,
   DECL_EXTERNAL (fn) = true;
   DECL_NOT_REALLY_EXTERN (fn) = 1;
   DECL_DECLARED_INLINE_P (fn) = 1;
+  DECL_COMDAT (fn) = 1;
+  set_linkage_according_to_type (type, fn);
+  rest_of_decl_compilation (fn, toplevel_bindings_p (), at_eof);
   gcc_assert (!TREE_USED (fn));
 
   /* Restore PROCESSING_TEMPLATE_DECL.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b32cf6c..0eac771 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -2787,6 +2787,9 @@ check_explicit_specialization (tree declarator,
 	       It's just the name of an instantiation.  But, it's not
 	       a request for an instantiation, either.  */
 	    SET_DECL_IMPLICIT_INSTANTIATION (decl);
+	  else
+	    /* A specialization is not necessarily COMDAT.  */
+	    DECL_COMDAT (decl) = DECL_DECLARED_INLINE_P (decl);
 
 	  /* Register this specialization so that we can find it
 	     again.  */
@@ -5017,6 +5020,13 @@ template arguments to %qD do not match original template %qD",
 	DECL_TEMPLATE_INFO (decl) = info;
     }
 
+  if (flag_implicit_templates
+      && VAR_OR_FUNCTION_DECL_P (decl))
+    /* Set DECL_COMDAT on template instantiations; if we force
+       them to be emitted by explicit instantiation or -frepo,
+       mark_needed will tell cgraph to do the right thing.  */
+    DECL_COMDAT (decl) = true;
+
   return DECL_TEMPLATE_RESULT (tmpl);
 }
 
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index f6c5693..1bfffb8 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3722,23 +3722,15 @@ decl_linkage (tree decl)
   if (TREE_CODE (decl) == CONST_DECL)
     return decl_linkage (TYPE_NAME (DECL_CONTEXT (decl)));
 
-  /* Some things that are not TREE_PUBLIC have external linkage, too.
-     For example, on targets that don't have weak symbols, we make all
-     template instantiations have internal linkage (in the object
-     file), but the symbols should still be treated as having external
-     linkage from the point of view of the language.  */
-  if (VAR_OR_FUNCTION_DECL_P (decl)
-      && DECL_COMDAT (decl))
-    return lk_external;
-
   /* Things in local scope do not have linkage, if they don't have
      TREE_PUBLIC set.  */
   if (decl_function_context (decl))
     return lk_none;
 
   /* Members of the anonymous namespace also have TREE_PUBLIC unset, but
-     are considered to have external linkage for language purposes.  DECLs
-     really meant to have internal linkage have DECL_THIS_STATIC set.  */
+     are considered to have external linkage for language purposes, as do
+     template instantiations on targets without weak symbols.  DECLs really
+     meant to have internal linkage have DECL_THIS_STATIC set.  */
   if (TREE_CODE (decl) == TYPE_DECL)
     return lk_external;
   if (VAR_OR_FUNCTION_DECL_P (decl))
diff --git a/gcc/testsuite/g++.dg/opt/devirt4.C b/gcc/testsuite/g++.dg/opt/devirt4.C
index 5a24eec..72f56af 100644
--- a/gcc/testsuite/g++.dg/opt/devirt4.C
+++ b/gcc/testsuite/g++.dg/opt/devirt4.C
@@ -1,8 +1,7 @@
 // PR lto/53808
-// Devirtualization + inlining should produce a non-virtual
-// call to ~foo.
-// { dg-options "-O -fdevirtualize" }
-// { dg-final { scan-assembler "_ZN3fooD2Ev" } }
+// Devirtualization should not produce an external ref to ~bar.
+// { dg-options "-O2" }
+// { dg-final { scan-assembler-not "_ZN3barD0Ev" } }
 
 struct foo {
  virtual ~foo();

commit 39bc88175548244f3eadde76c506dbace3a4e463
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jul 29 12:42:34 2014 -0400

    	PR c++/61659
    	PR c++/61687
    Revert:
    gcc/c-family/
    	* c.opt (-fuse-all-virtuals): New.
    gcc/cp/
    	* decl2.c (mark_all_virtuals): New variable.
    	(maybe_emit_vtables): Check it instead of flag_devirtualize.
    	(cp_write_global_declarations): Set it and give helpful diagnostic
    	if it introduces errors.
    	* class.c (finish_struct_1): Check it.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index ac3eb44..f427da1 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1276,10 +1276,6 @@ funsigned-char
 C ObjC C++ ObjC++ LTO Var(flag_signed_char, 0)
 Make \"char\" unsigned by default
 
-fuse-all-virtuals
-C++ ObjC++ Var(flag_use_all_virtuals) Init(1)
-Treat all virtual functions as odr-used
-
 fuse-cxa-atexit
 C++ ObjC++ Var(flag_use_cxa_atexit) Init(DEFAULT_USE_CXA_ATEXIT)
 Use __cxa_atexit to register destructors
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 0f611e1..235c68a 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -6408,7 +6408,7 @@ finish_struct_1 (tree t)
 	 in every translation unit where the class definition appears.  If
 	 we're devirtualizing, we can look into the vtable even if we
 	 aren't emitting it.  */
-      if (CLASSTYPE_KEY_METHOD (t) == NULL_TREE || flag_use_all_virtuals)
+      if (CLASSTYPE_KEY_METHOD (t) == NULL_TREE)
 	keyed_classes = tree_cons (NULL_TREE, t, keyed_classes);
     }
 
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 884be0a..eafdce5 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -106,11 +106,6 @@ static GTY(()) vec<tree, va_gc> *no_linkage_decls;
 /* Nonzero if we're done parsing and into end-of-file activities.  */
 
 int at_eof;
-
-/* Nonzero if we've instantiated everything used directly, and now want to
-   mark all virtual functions as used so that they are available for
-   devirtualization.  */
-static int mark_all_virtuals;
 
 
 /* Return a member function type (a METHOD_TYPE), given FNTYPE (a
@@ -2020,15 +2015,6 @@ maybe_emit_vtables (tree ctype)
       if (DECL_COMDAT (primary_vtbl)
 	  && CLASSTYPE_DEBUG_REQUESTED (ctype))
 	note_debug_info_needed (ctype);
-      if (mark_all_virtuals && !DECL_ODR_USED (primary_vtbl))
-	{
-	  /* Make sure virtual functions get instantiated/synthesized so that
-	     they can be inlined after devirtualization even if the vtable is
-	     never emitted.  */
-	  mark_used (primary_vtbl);
-	  mark_vtable_entries (primary_vtbl);
-	  return true;
-	}
       return false;
     }
 
@@ -4345,8 +4331,6 @@ cp_write_global_declarations (void)
      instantiated, etc., etc.  */
 
   emit_support_tinfos ();
-  int errs = errorcount + sorrycount;
-  bool explained_devirt = false;
 
   do
     {
@@ -4579,27 +4563,6 @@ cp_write_global_declarations (void)
 					 pending_statics->length ()))
 	reconsider = true;
 
-      if (flag_use_all_virtuals)
-	{
-	  if (!reconsider && !mark_all_virtuals)
-	    {
-	      mark_all_virtuals = true;
-	      reconsider = true;
-	      errs = errorcount + sorrycount;
-	    }
-	  else if (mark_all_virtuals
-		   && !explained_devirt
-		   && (errorcount + sorrycount > errs))
-	    {
-	      inform (global_dc->last_location, "this error is seen due to "
-		      "instantiation of all virtual functions, which the C++ "
-		      "standard says are always considered used; this is done "
-		      "to support devirtualization optimizations, but can be "
-		      "disabled with -fno-use-all-virtuals");
-	      explained_devirt = true;
-	    }
-	}
-
       retries++;
     }
   while (reconsider);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5dfa586..51757f0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -189,7 +189,7 @@ in the following sections.
 -fno-pretty-templates @gol
 -frepo  -fno-rtti  -fstats  -ftemplate-backtrace-limit=@var{n} @gol
 -ftemplate-depth=@var{n} @gol
--fno-threadsafe-statics  -fno-use-all-virtuals  -fuse-cxa-atexit @gol
+-fno-threadsafe-statics  -fuse-cxa-atexit @gol
 -fno-weak  -nostdinc++ @gol
 -fvisibility-inlines-hidden @gol
 -fvtable-verify=@var{std|preinit|none} @gol
@@ -2319,16 +2319,6 @@ ABI for thread-safe initialization of local statics.  You can use this
 option to reduce code size slightly in code that doesn't need to be
 thread-safe.
 
-@item -fno-use-all-virtuals
-@opindex fno-use-all-virtuals
-By default, G++ now treats all virtual functions declared in a
-translation unit as odr-used, so they will be instantiated or
-synthesized if possible even if they are not needed for the final
-output.  This is done so that such functions can be inlined after
-devirtualization changes an indirect call into a direct call.  If this
-instantiation and synthesis prevents your code from compiling
-successfully, you can disable it with this option.
-
 @item -fuse-cxa-atexit
 @opindex fuse-cxa-atexit
 Register destructors for objects with static storage duration with the
diff --git a/gcc/testsuite/g++.dg/template/dtor9.C b/gcc/testsuite/g++.dg/template/dtor9.C
index 006a754..fd71389 100644
--- a/gcc/testsuite/g++.dg/template/dtor9.C
+++ b/gcc/testsuite/g++.dg/template/dtor9.C
@@ -1,5 +1,4 @@
 // PR c++/60347
-// { dg-options "-fno-use-all-virtuals" }
 
 struct A;
 
diff --git a/gcc/testsuite/g++.dg/template/dtor9a.C b/gcc/testsuite/g++.dg/template/dtor9a.C
deleted file mode 100644
index aaae8b6..0000000
--- a/gcc/testsuite/g++.dg/template/dtor9a.C
+++ /dev/null
@@ -1,13 +0,0 @@
-// PR c++/60347
-// { dg-options "-fuse-all-virtuals" }
-
-struct A;
-
-template <class T>
-struct B
-{
-  T* p;
-  virtual ~B() { p->~T(); }	// { dg-error "incomplete" }
-};
-
-struct C: B<A> { };