diff mbox series

Fix divergence of profile data with -flto and without

Message ID 20181227212952.z56cpaypr4kq6tan@kam.mff.cuni.cz
State New
Headers show
Series Fix divergence of profile data with -flto and without | expand

Commit Message

Jan Hubicka Dec. 27, 2018, 9:29 p.m. UTC 
Hi,
Firefox was patched to build train run w/o LTO and final build with LTO.
This causes problems where LTO and non-LTO builds diverge during early
optimization. This can happen because free_lang_data clears type
inheritance graph which in turn is rebuilt taking into account what
virtual tables has been optimized out and leads to better
devirutalization. This seems to trigger surprisinly often for Firefox
and is fixed by rebuilding it all the time.

While reducing (a gigantic) testcase I ran into ICE which is fixed too.
Dumping code has sanity check that speculative targets are subset of
full targets list and this condition is broken in a specil case where
new type is introduced into the type inheritnace graph later and one of
lists is cached from time it was not there.

I can not clear the cache each time new type is introduced because
passes use cache addresses to avoid redundant work on lists which was
seen prevoiusly, but it is easy to avoid re-use of the old entries when
this hapens.

Bootstrapped/regtested x86_64-linux, commited to mainline. I plan to
backport this to release branches after few days.
I still see some profile mismatches on Firefox and I am investigating
whether it is difference on firefox or gcc side.

Honza

	* ipa-devirt.c (polymorphic_call_target_d): Add n_odr_types.
	(polymorphic_call_target_hasher::hash): Hash it.
	(polymorphic_call_target_hasher::equal): Compare it.
	(possible_polymorphic_call_targets): Set it.
	* tree.c (free_lang_data): Rebuild type inheritance graph even on
	non-LTO path.

	* g++.dg/ipa/devirt-53.C: New testcase.
diff mbox series

Patch

Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 267432)
+++ ipa-devirt.c	(working copy)
@@ -2759,6 +2758,7 @@  struct polymorphic_call_target_d
   vec <cgraph_node *> targets;
   tree decl_warning;
   int type_warning;
+  unsigned int n_odr_types;
   bool complete;
   bool speculative;
 };
@@ -2784,6 +2784,7 @@  polymorphic_call_target_hasher::hash (co
   hstate.add_hwi (odr_query->type->id);
   hstate.merge_hash (TYPE_UID (odr_query->context.outer_type));
   hstate.add_hwi (odr_query->context.offset);
+  hstate.add_hwi (odr_query->n_odr_types);
 
   if (odr_query->context.speculative_outer_type)
     {
@@ -2814,7 +2815,9 @@  polymorphic_call_target_hasher::equal (c
 	      == t2->context.maybe_in_construction
 	  && t1->context.maybe_derived_type == t2->context.maybe_derived_type
 	  && (t1->context.speculative_maybe_derived_type
-	      == t2->context.speculative_maybe_derived_type));
+	      == t2->context.speculative_maybe_derived_type)
+	  /* Adding new type may affect outcome of target search.  */
+	  && t1->n_odr_types == t2->n_odr_types);
 }
 
 /* Remove entry in polymorphic call target cache hash.  */
@@ -3220,6 +3223,7 @@  possible_polymorphic_call_targets (tree
   key.otr_token = otr_token;
   key.speculative = speculative;
   key.context = context;
+  key.n_odr_types = odr_types.length ();
   slot = polymorphic_call_target_hash->find_slot (&key, INSERT);
   if (cache_token)
    *cache_token = (void *)*slot;
@@ -3436,6 +3440,7 @@  possible_polymorphic_call_targets (tree
 
   (*slot)->targets = nodes;
   (*slot)->complete = complete;
+  (*slot)->n_odr_types = odr_types.length ();
   if (completep)
     *completep = complete;
 
Index: testsuite/g++.dg/ipa/devirt-53.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-53.C	(nonexistent)
+++ testsuite/g++.dg/ipa/devirt-53.C	(working copy)
@@ -0,0 +1,58 @@ 
+// { dg-do assemble }
+// { dg-options "-O2 -fdump-tree-fre1-details -std=c++11 -Wno-return-type" }
+typedef unsigned a;
+enum b : a;
+class c {
+public:
+  virtual a d();
+};
+using e = int;
+class f;
+class h {
+public:
+  f *operator->();
+};
+class i {
+public:
+  ~i() { j->d(); }
+  c *j;
+};
+template <class g> class k : i {
+public:
+  k(g *);
+};
+class l;
+class m {
+  virtual b n(const e &, l **);
+};
+class o {
+protected:
+  h p;
+};
+class G {
+  virtual b r(const e &, l **);
+};
+class l : G {};
+class q {
+public:
+  q(l *);
+  template <class t> void s(t);
+};
+class f : c {
+  a d();
+  virtual b r(e);
+
+public:
+  class L : public l, o, m {
+    b r(const e &y, l **) { p->r(y); }
+    b n(const e &, l **) { k<l> a = this; }
+  };
+};
+c u;
+void fn1() {
+  c v;
+  k<c> b(&u);
+  q(new f::L).s(v);
+}
+/* Check that f::d appears as possible target.  */
+/* { dg-final { scan-tree-dump "f::d" "fre"  } } */
Index: tree.c
===================================================================
--- tree.c	(revision 267432)
+++ tree.c	(working copy)
@@ -6191,7 +6191,12 @@  free_lang_data (void)
   /* If we are the LTO frontend we have freed lang-specific data already.  */
   if (in_lto_p
       || (!flag_generate_lto && !flag_generate_offload))
-    return 0;
+    {
+      /* Rebuild type inheritance graph even when not doing LTO to get
+	 consistent profile data.  */
+      rebuild_type_inheritance_graph ();
+      return 0;
+    }
 
   fld_incomplete_types = new hash_map<tree, tree>;
   fld_simplified_types = new hash_map<tree, tree>;