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 <ja...@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 <ja...@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> { };
