This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: C++ PATCH for c++/61687 (extra errors with -O2)


On 07/24/2014 07:36 PM, Jan Hubicka wrote:
ipa-deivrt has code to do the tracking for you. All is needed is to cal get_odr_type
for all polymorphic type that we care about.  build_type_inheritance_graph just
walks all virtual methods to register all polymorphic types that matters (i.e.
have methods associated with them) into the datastructure.  I think within C++
FE we can just reigster all polymoprhic types we produce.

Makes sense.

I see that during the reachability walk new types may be instantiated. This may
extend target lists earlier visited, I guess we need to add some way to make
to track this?

True.

And more problematic, doing this in a single TU isn't good enough; there are cases where LTO can devirtualize a call to a target that isn't declared in the TU where the call occurs. That is,

foo.h:
struct A { virtual ~A() {} };
void g(A*);

bar.h:
#include "foo.h"
struct B: A { virtual void f(); };

one.C:
#include "foo.h"
void g(A* p) { delete p; }

two.C:
#include "bar.h"
int main() { g(new B); }

three.C:
#include "bar.h"
void B::f() {}

$ gcc -fpic -shared -fvisibility-inlines-hidden three.C -o libfoo.so
$ gcc -O2 -flto -fvisibility-inlines-hidden one.C two.C libfoo.so
/home/jason/gtt/foo/one.C:2: undefined reference to `B::~B()'

To deal with this I guess we can either keep -fuse-all-virtuals or not devirtualize in LTO to something that is hidden and not defined. Thoughts?

commit ac04d4b08190593299c8c94431d5e43384514096
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jul 25 09:15:02 2014 -0400

    mark_virtual_overrides

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c318cad..75711a5 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1272,10 +1272,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/call.c b/gcc/cp/call.c
index 4d37c65..969e730 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7364,6 +7364,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 				ba_any, NULL, complain);
       gcc_assert (binfo && binfo != error_mark_node);
 
+      note_fn_called_virtually (fn);
+
       /* Warn about deprecated virtual functions now, since we're about
 	 to throw away the decl.  */
       if (TREE_DEPRECATED (fn))
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 0f611e1..1401069 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -817,6 +817,9 @@ get_vtable_decl (tree type, int complete)
   decl = build_vtable (type, get_vtable_name (type), vtbl_type_node);
   CLASSTYPE_VTABLES (type) = decl;
 
+  /* Make the vtable visible to build_type_inheritance_graph.  */
+  varpool_node::get_create (decl);
+
   if (complete)
     {
       DECL_EXTERNAL (decl) = 1;
@@ -6408,7 +6411,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/cp-tree.h b/gcc/cp/cp-tree.h
index 0c0d804..c9f248a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -109,6 +109,7 @@ c-common.h, not after.
       BIND_EXPR_BODY_BLOCK (in BIND_EXPR)
       DECL_NON_TRIVIALLY_INITIALIZED_P (in VAR_DECL)
       CALL_EXPR_LIST_INIT_P (in CALL_EXPR, AGGR_INIT_EXPR)
+      FNDECL_CALLED_VIRTUALLY (in FUNCTION_DECL)
    4: TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
 	  or FIELD_DECL).
       IDENTIFIER_TYPENAME_P (in IDENTIFIER_NODE)
@@ -3222,6 +3223,11 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
 #define FNDECL_USED_AUTO(NODE) \
   TREE_LANG_FLAG_2 (FUNCTION_DECL_CHECK (NODE))
 
+/* True if NODE was called through the vtable; used to avoid duplicates in
+   fns_called_virtually.  */
+#define FNDECL_CALLED_VIRTUALLY(NODE) \
+  TREE_LANG_FLAG_3 (FUNCTION_DECL_CHECK (NODE))
+
 /* Nonzero if NODE is a DECL which we know about but which has not
    been explicitly declared, such as a built-in function or a friend
    declared inside a class.  In the latter case DECL_HIDDEN_FRIEND_P
@@ -5381,6 +5387,7 @@ extern tree get_tls_wrapper_fn			(tree);
 extern void mark_needed				(tree);
 extern bool decl_needed_p			(tree);
 extern void note_vague_linkage_fn		(tree);
+extern void note_fn_called_virtually		(tree);
 extern tree build_artificial_parm		(tree, tree);
 extern bool possibly_inlined_p			(tree);
 extern int parm_index                           (tree);
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 8fa3145..a448103 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -47,6 +47,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-common.h"
 #include "c-family/c-objc.h"
 #include "cgraph.h"
+/* For build_type_inheritance_graph and possible_polymorphic_call_targets.  */
+#include "ipa-utils.h"
 #include "tree-inline.h"
 #include "c-family/c-pragma.h"
 #include "dumpfile.h"
@@ -103,6 +105,10 @@ static GTY(()) vec<tree, va_gc> *deferred_fns;
    sure are defined.  */
 static GTY(()) vec<tree, va_gc> *no_linkage_decls;
 
+/* A list of functions called through the vtable, so we can mark their
+   overriders as used.  */
+static GTY(()) vec<tree, va_gc> *fns_called_virtually;
+
 /* Nonzero if we're done parsing and into end-of-file activities.  */
 
 int at_eof;
@@ -791,6 +797,19 @@ note_vague_linkage_fn (tree decl)
   vec_safe_push (deferred_fns, decl);
 }
 
+/* DECL is a function being called through the vtable.  Remember it so that
+   at the end of the translation unit we can mark as used any functions
+   that override it, for devirtualization.  */
+
+void
+note_fn_called_virtually (tree decl)
+{
+  if (FNDECL_CALLED_VIRTUALLY (decl))
+    return;
+  FNDECL_CALLED_VIRTUALLY (decl) = true;
+  vec_safe_push (fns_called_virtually, decl);
+}
+
 /* We have just processed the DECL, which is a static data member.
    The other parameters are as for cp_finish_decl.  */
 
@@ -4281,6 +4300,39 @@ dump_tu (void)
     }
 }
 
+/* Now that we've seen all the types in the translation unit, go back over
+   the list of functions called through the vtable and mark any overriders
+   as used so they're available for devirtualization.  */
+
+static bool
+mark_virtual_overrides (void)
+{
+  if (!fns_called_virtually)
+    return false;
+
+  build_type_inheritance_graph ();
+
+  bool reconsider = false;
+  size_t i; tree fn;
+  FOR_EACH_VEC_SAFE_ELT (fns_called_virtually, i, fn)
+    {
+      vec<cgraph_node*> targets
+	= possible_polymorphic_call_targets (fn);
+
+      size_t j; cgraph_node *n;
+      FOR_EACH_VEC_ELT (targets, j, n)
+	{
+	  if (!DECL_ODR_USED (n->decl))
+	    reconsider = true;
+	  mark_used (n->decl);
+	}
+    }
+
+  release_tree_vector (fns_called_virtually);
+  fns_called_virtually = NULL;
+  return reconsider;
+}
+
 /* This routine is called at the end of compilation.
    Its job is to create all the code needed to initialize and
    destroy the global aggregates.  We do the destruction
@@ -4349,8 +4401,6 @@ cp_write_global_declarations (void)
      instantiated, etc., etc.  */
 
   emit_support_tinfos ();
-  int errs = errorcount + sorrycount;
-  bool explained_devirt = false;
 
   do
     {
@@ -4396,6 +4446,9 @@ cp_write_global_declarations (void)
 	    }
 	}
 
+      if (mark_virtual_overrides ())
+	reconsider = true;
+
       /* Write out needed type info variables.  We have to be careful
 	 looping through unemitted decls, because emit_tinfo_decl may
 	 cause other variables to be needed. New elements will be
@@ -4583,27 +4636,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/ipa-devirt.c b/gcc/ipa-devirt.c
index 7c4151a..2bb6461 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -2592,6 +2592,17 @@ possible_polymorphic_call_targets (tree otr_type,
   return nodes;
 }
 
+/* Return vector containing possible targets of polymorphic call to
+   function FN.  */
+
+vec <cgraph_node *>
+possible_polymorphic_call_targets (tree fn)
+{
+  return possible_polymorphic_call_targets
+    (DECL_CONTEXT (fn), tree_to_uhwi (DECL_VINDEX (fn)),
+     ipa_dummy_polymorphic_call_context);
+}
+
 /* Dump all possible targets of a polymorphic call.  */
 
 void
diff --git a/gcc/ipa-utils.h b/gcc/ipa-utils.h
index bb2e0d5..1023794 100644
--- a/gcc/ipa-utils.h
+++ b/gcc/ipa-utils.h
@@ -78,6 +78,7 @@ possible_polymorphic_call_targets (tree, HOST_WIDE_INT,
 				   bool *final = NULL,
 				   void **cache_token = NULL,
 				   int *nonconstruction_targets = NULL);
+vec <cgraph_node *> possible_polymorphic_call_targets (tree fn);
 odr_type get_odr_type (tree, bool insert = false);
 void dump_possible_polymorphic_call_targets (FILE *, tree, HOST_WIDE_INT,
 					     const ipa_polymorphic_call_context &);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]