[PATCH][?/n] Cleanup LTO type merging

[PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

Hashes of SCC members have been poor since ever as we have to ensure
they are the same when the SCC is entered from a different path.  But
we can do better and mixin SCC member hashes if we can sort the SCC
in some way.  And we can do so, when sorting after the hash values.
The only thing we need to ensure is that we ignore members with
the same hash value during mixin.

The following implements this.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2011-05-16  Richard Guenther  <rguenther@suse.de>

	* gimple.c (struct type_hash_pair): New type.
	(type_hash_pair_compare): New function.
	(iterative_hash_gimple_type): Mix in SCC member hashes in
	hash-order.

Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c	(revision 173732)
+++ gcc/gimple.c	(working copy)
@@ -4056,6 +4056,25 @@ iterative_hash_name (tree name, hashval_
   return iterative_hash_object (IDENTIFIER_HASH_VALUE (name), v);
 }
 
+/* A type, hashvalue pair for sorting SCC members.  */
+
+struct type_hash_pair {
+  tree type;
+  hashval_t hash;
+};
+
+/* Compare two type, hashvalue pairs.  */
+
+static int
+type_hash_pair_compare (const void *p1_, const void *p2_)
+{
+  const struct type_hash_pair *p1 = (const struct type_hash_pair *) p1_;
+  const struct type_hash_pair *p2 = (const struct type_hash_pair *) p2_;
+  if (p1->hash == p2->hash)
+    return TYPE_UID (p1->type) - TYPE_UID (p2->type);
+  return p1->hash - p2->hash;
+}
+
 /* Returning a hash value for gimple type TYPE combined with VAL.
    SCCSTACK, SCCSTATE and SCCSTATE_OBSTACK are state for the DFS walk done.
 
@@ -4220,22 +4213,73 @@ iterative_hash_gimple_type (tree type, h
   if (state->low == state->dfsnum)
     {
       tree x;
+      struct sccs *cstate;
+      struct tree_int_map *m;
 
       /* Pop off the SCC and set its hash values.  */
-      do
+      x = VEC_pop (tree, *sccstack);
+      cstate = (struct sccs *)*pointer_map_contains (sccstate, x);
+      cstate->on_sccstack = false;
+      /* Optimize SCC size one.  */
+      if (x == type)
 	{
-	  struct sccs *cstate;
-	  struct tree_int_map *m = ggc_alloc_cleared_tree_int_map ();
-	  x = VEC_pop (tree, *sccstack);
-	  cstate = (struct sccs *)*pointer_map_contains (sccstate, x);
-	  cstate->on_sccstack = false;
+	  m = ggc_alloc_cleared_tree_int_map ();
 	  m->base.from = x;
 	  m->to = cstate->u.hash;
 	  slot = htab_find_slot (type_hash_cache, m, INSERT);
 	  gcc_assert (!*slot);
 	  *slot = (void *) m;
 	}
-      while (x != type);
+      else
+	{
+	  unsigned first, i, size, j;
+	  struct type_hash_pair *pairs;
+	  /* Pop off the SCC and build an array of type, hash pairs.  */
+	  first = VEC_length (tree, *sccstack) - 1;
+	  while (VEC_index (tree, *sccstack, first) != type)
+	    --first;
+	  size = VEC_length (tree, *sccstack) - first + 1;
+	  pairs = XALLOCAVEC (struct type_hash_pair, size);
+	  i = 0;
+	  pairs[i].type = x;
+	  pairs[i].hash = cstate->u.hash;
+	  do
+	    {
+	      x = VEC_pop (tree, *sccstack);
+	      cstate = (struct sccs *)*pointer_map_contains (sccstate, x);
+	      cstate->on_sccstack = false;
+	      ++i;
+	      pairs[i].type = x;
+	      pairs[i].hash = cstate->u.hash;
+	    }
+	  while (x != type);
+	  gcc_assert (i + 1 == size);
+	  /* Sort the arrays of type, hash pairs so that when we mix in
+	     all members of the SCC the hash value becomes independent on
+	     the order we visited the SCC.  Disregard hashes equal to
+	     the hash of the type we mix into because we cannot guarantee
+	     a stable sort for those across different TUs.  */
+	  qsort (pairs, size, sizeof (struct type_hash_pair),
+		 type_hash_pair_compare);
+	  for (i = 0; i < size; ++i)
+	    {
+	      hashval_t hash;
+	      m = ggc_alloc_cleared_tree_int_map ();
+	      m->base.from = pairs[i].type;
+	      hash = pairs[i].hash;
+	      /* Skip same hashes.  */
+	      for (j = i + 1; j < size && pairs[j].hash == pairs[i].hash; ++j)
+		;
+	      for (; j < size; ++j)
+		hash = iterative_hash_hashval_t (pairs[j].hash, hash);
+	      for (j = 0; pairs[j].hash != pairs[i].hash; ++j)
+		hash = iterative_hash_hashval_t (pairs[j].hash, hash);
+	      m->to = hash;
+	      slot = htab_find_slot (type_hash_cache, m, INSERT);
+	      gcc_assert (!*slot);
+	      *slot = (void *) m;
+	    }
+	}
     }
 
   return iterative_hash_hashval_t (v, val);

[PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

This moves all pointer-to, main-variant and canonical-type setup
to a central place which happens after we fixed up and registered
types for a set of new tree nodes.  In particular delaying canonical
type computation after setting up the pointer-to chains should
enable a fix for PR48849 and its eventual duplicates.

LTO bootstrapped and tested on x86_64-unknown-linux-gnu, kernel WPA
tested, SPEC2k6 LTO build and test in progress.

Richard.

2011-05-19  Richard Guenther  <rguenther@suse.de>

	* gimple.c (gimple_register_type_1): Do not fiddle with
	main-variant or pointer-to chains.  Delay all fixup to
	uniquify_nodes.

	lto/
	* lto.c (lto_ft_common): Remove pointer-to chain teardown.
	(lto_ft_type): Move main-variant and pointer-to chain building ...
	(uniquify_nodes): ... here.  Compute TYPE_CANONICAL also here,
	in a separate final loop.


Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 173900)
+++ gcc/lto/lto.c	(working copy)
@@ -259,45 +259,9 @@ static void lto_fixup_types (tree);
 static void
 lto_ft_common (tree t)
 {
-  /* The following re-creates the TYPE_REFERENCE_TO and TYPE_POINTER_TO
-     lists.  We do not stream TYPE_REFERENCE_TO, TYPE_POINTER_TO or
-     TYPE_NEXT_PTR_TO and TYPE_NEXT_REF_TO.
-     First remove us from any pointer list we are on.  */
-  if (TREE_CODE (t) == POINTER_TYPE)
-    {
-      if (TYPE_POINTER_TO (TREE_TYPE (t)) == t)
-	TYPE_POINTER_TO (TREE_TYPE (t)) = TYPE_NEXT_PTR_TO (t);
-      else
-	{
-	  tree tem = TYPE_POINTER_TO (TREE_TYPE (t));
-	  while (tem && TYPE_NEXT_PTR_TO (tem) != t)
-	    tem = TYPE_NEXT_PTR_TO (tem);
-	  if (tem)
-	    TYPE_NEXT_PTR_TO (tem) = TYPE_NEXT_PTR_TO (t);
-	}
-      TYPE_NEXT_PTR_TO (t) = NULL_TREE;
-    }
-  else if (TREE_CODE (t) == REFERENCE_TYPE)
-    {
-      if (TYPE_REFERENCE_TO (TREE_TYPE (t)) == t)
-	TYPE_REFERENCE_TO (TREE_TYPE (t)) = TYPE_NEXT_REF_TO (t);
-      else
-	{
-	  tree tem = TYPE_REFERENCE_TO (TREE_TYPE (t));
-	  while (tem && TYPE_NEXT_REF_TO (tem) != t)
-	    tem = TYPE_NEXT_REF_TO (tem);
-	  if (tem)
-	    TYPE_NEXT_REF_TO (tem) = TYPE_NEXT_REF_TO (t);
-	}
-      TYPE_NEXT_REF_TO (t) = NULL_TREE;
-    }
-
   /* Fixup our type.  */
   LTO_FIXUP_TREE (TREE_TYPE (t));
 
-  /* Second put us on the list of pointers of the new pointed-to type
-     if we are a main variant.  This is done in lto_ft_type after
-     fixing up our main variant.  */
   LTO_FIXUP_TREE (TREE_CHAIN (t));
 }
 
@@ -374,8 +338,6 @@ lto_ft_field_decl (tree t)
 static void
 lto_ft_type (tree t)
 {
-  tree tem, mv;
-
   lto_ft_common (t);
   LTO_FIXUP_TREE (TYPE_CACHED_VALUES (t));
   LTO_FIXUP_TREE (TYPE_SIZE (t));
@@ -392,67 +354,6 @@ lto_ft_type (tree t)
   LTO_FIXUP_TREE (t->type_non_common.binfo);
 
   LTO_FIXUP_TREE (TYPE_CONTEXT (t));
-
-  /* Compute the canonical type of t and fix that up.  From this point
-     there are no longer any types with TYPE_STRUCTURAL_EQUALITY_P
-     and its type-based alias problems.  */
-  if (!TYPE_CANONICAL (t))
-    {
-      TYPE_CANONICAL (t) = gimple_register_canonical_type (t);
-      LTO_FIXUP_TREE (TYPE_CANONICAL (t));
-    }
-
-  /* The following re-creates proper variant lists while fixing up
-     the variant leaders.  We do not stream TYPE_NEXT_VARIANT so the
-     variant list state before fixup is broken.  */
-
-  /* Remove us from our main variant list if we are not the variant leader.  */
-  if (TYPE_MAIN_VARIANT (t) != t)
-    {
-      tem = TYPE_MAIN_VARIANT (t);
-      while (tem && TYPE_NEXT_VARIANT (tem) != t)
-	tem = TYPE_NEXT_VARIANT (tem);
-      if (tem)
-	TYPE_NEXT_VARIANT (tem) = TYPE_NEXT_VARIANT (t);
-      TYPE_NEXT_VARIANT (t) = NULL_TREE;
-    }
-
-  /* Query our new main variant.  */
-  mv = gimple_register_type (TYPE_MAIN_VARIANT (t));
-
-  /* If we were the variant leader and we get replaced ourselves drop
-     all variants from our list.  */
-  if (TYPE_MAIN_VARIANT (t) == t
-      && mv != t)
-    {
-      tem = t;
-      while (tem)
-	{
-	  tree tem2 = TYPE_NEXT_VARIANT (tem);
-	  TYPE_NEXT_VARIANT (tem) = NULL_TREE;
-	  tem = tem2;
-	}
-    }
-
-  /* Finally adjust our main variant and fix it up.  */
-  TYPE_MAIN_VARIANT (t) = mv;
-  LTO_FIXUP_TREE (TYPE_MAIN_VARIANT (t));
-
-  /* As the second step of reconstructing the pointer chains put us
-     on the list of pointers of the new pointed-to type
-     if we are a main variant.  See lto_ft_common for the first step.  */
-  if (TREE_CODE (t) == POINTER_TYPE
-      && TYPE_MAIN_VARIANT (t) == t)
-    {
-      TYPE_NEXT_PTR_TO (t) = TYPE_POINTER_TO (TREE_TYPE (t));
-      TYPE_POINTER_TO (TREE_TYPE (t)) = t;
-    }
-  else if (TREE_CODE (t) == REFERENCE_TYPE
-	   && TYPE_MAIN_VARIANT (t) == t)
-    {
-      TYPE_NEXT_REF_TO (t) = TYPE_REFERENCE_TO (TREE_TYPE (t));
-      TYPE_REFERENCE_TO (TREE_TYPE (t)) = t;
-    }
 }
 
 /* Fix up fields of a BINFO T.  */
@@ -608,19 +509,21 @@ uniquify_nodes (struct data_in *data_in,
 
   /* Go backwards because childs streamed for the first time come
      as part of their parents, and hence are created after them.  */
+
+  /* First register all types in the cache.
+     This makes sure to have the original structure in the type cycles
+     when registering them and computing hashes.  */
   for (i = len; i-- > from;)
     {
       tree t = VEC_index (tree, cache->nodes, i);
-      if (!t)
+      if (!t
+	  || !TYPE_P (t))
 	continue;
 
-      /* Now try to find a canonical variant of T itself.  */
-      if (TYPE_P (t))
-	gimple_register_type (t);
+      gimple_register_type (t);
     }
 
-  /* Go backwards because childs streamed for the first time come
-     as part of their parents, and hence are created after them.  */
+  /* Second fixup all trees in the new cache entries.  */
   for (i = len; i-- > from;)
     {
       tree t = VEC_index (tree, cache->nodes, i);
@@ -631,43 +534,103 @@ uniquify_nodes (struct data_in *data_in,
       /* First fixup the fields of T.  */
       lto_fixup_types (t);
 
+      if (!TYPE_P (t))
+	continue;
+
       /* Now try to find a canonical variant of T itself.  */
-      if (TYPE_P (t))
+      t = gimple_register_type (t);
+
+      if (t == oldt)
 	{
-	  t = gimple_register_type (t);
-	  if (t == oldt
-	      && TYPE_MAIN_VARIANT (t) != t)
+	  /* The following re-creates proper variant lists while fixing up
+	     the variant leaders.  We do not stream TYPE_NEXT_VARIANT so the
+	     variant list state before fixup is broken.  */
+	  tree tem, mv;
+
+	  /* Remove us from our main variant list if we are not the
+	     variant leader.  */
+	  if (TYPE_MAIN_VARIANT (t) != t)
 	    {
-	      /* If this is its own type, link it into the variant chain.  */
-	      TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (TYPE_MAIN_VARIANT (t));
-	      TYPE_NEXT_VARIANT (TYPE_MAIN_VARIANT (t)) = t;
+	      tem = TYPE_MAIN_VARIANT (t);
+	      while (tem && TYPE_NEXT_VARIANT (tem) != t)
+		tem = TYPE_NEXT_VARIANT (tem);
+	      if (tem)
+		TYPE_NEXT_VARIANT (tem) = TYPE_NEXT_VARIANT (t);
+	      TYPE_NEXT_VARIANT (t) = NULL_TREE;
 	    }
-	}
-      if (t != oldt)
-	{
-	  if (RECORD_OR_UNION_TYPE_P (t))
+
+	  /* Query our new main variant.  */
+	  mv = gimple_register_type (TYPE_MAIN_VARIANT (t));
+
+	  /* If we were the variant leader and we get replaced ourselves drop
+	     all variants from our list.  */
+	  if (TYPE_MAIN_VARIANT (t) == t
+	      && mv != t)
+	    {
+	      tem = t;
+	      while (tem)
+		{
+		  tree tem2 = TYPE_NEXT_VARIANT (tem);
+		  TYPE_NEXT_VARIANT (tem) = NULL_TREE;
+		  tem = tem2;
+		}
+	    }
+
+	  /* If we are not our own variant leader link us into our new leaders
+	     variant list.  */
+	  if (mv != t)
+	    {
+	      TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (mv);
+	      TYPE_NEXT_VARIANT (mv) = t;
+	    }
+
+	  /* Finally adjust our main variant and fix it up.  */
+	  TYPE_MAIN_VARIANT (t) = mv;
+
+	  /* The following reconstructs the pointer chains
+	     of the new pointed-to type if we are a main variant.  We do
+	     not stream those so they are broken before fixup.  */
+	  if (TREE_CODE (t) == POINTER_TYPE
+	      && TYPE_MAIN_VARIANT (t) == t)
+	    {
+	      TYPE_NEXT_PTR_TO (t) = TYPE_POINTER_TO (TREE_TYPE (t));
+	      TYPE_POINTER_TO (TREE_TYPE (t)) = t;
+	    }
+	  else if (TREE_CODE (t) == REFERENCE_TYPE
+		   && TYPE_MAIN_VARIANT (t) == t)
 	    {
-	      tree f1, f2;
-	      if (TYPE_FIELDS (t) != TYPE_FIELDS (oldt))
-		for (f1 = TYPE_FIELDS (t), f2 = TYPE_FIELDS (oldt);
-		     f1 && f2; f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
-		  {
-		    unsigned ix;
-		    gcc_assert (f1 != f2 && DECL_NAME (f1) == DECL_NAME (f2));
-		    if (!lto_streamer_cache_lookup (cache, f2, &ix))
-		      gcc_unreachable ();
-		    /* If we're going to replace an element which we'd
-		       still visit in the next iterations, we wouldn't
-		       handle it, so do it here.  We do have to handle it
-		       even though the field_decl itself will be removed,
-		       as it could refer to e.g. integer_cst which we
-		       wouldn't reach via any other way, hence they
-		       (and their type) would stay uncollected.  */
-		    if (ix < i)
-		      lto_fixup_types (f2);
-		    lto_streamer_cache_insert_at (cache, f1, ix);
-		  }
+	      TYPE_NEXT_REF_TO (t) = TYPE_REFERENCE_TO (TREE_TYPE (t));
+	      TYPE_REFERENCE_TO (TREE_TYPE (t)) = t;
 	    }
+	}
+
+      else if (RECORD_OR_UNION_TYPE_P (t))
+	{
+	  tree f1, f2;
+	  if (TYPE_FIELDS (t) != TYPE_FIELDS (oldt))
+	    for (f1 = TYPE_FIELDS (t), f2 = TYPE_FIELDS (oldt);
+		 f1 && f2; f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
+	      {
+		unsigned ix;
+		gcc_assert (f1 != f2 && DECL_NAME (f1) == DECL_NAME (f2));
+		if (!lto_streamer_cache_lookup (cache, f2, &ix))
+		  gcc_unreachable ();
+		/* If we're going to replace an element which we'd
+		   still visit in the next iterations, we wouldn't
+		   handle it, so do it here.  We do have to handle it
+		   even though the field_decl itself will be removed,
+		   as it could refer to e.g. integer_cst which we
+		   wouldn't reach via any other way, hence they
+		   (and their type) would stay uncollected.  */
+		/* ???  We should rather make sure to replace all
+		   references to f2 with f1.  That means handling
+		   COMPONENT_REFs and CONSTRUCTOR elements in
+		   lto_fixup_types and special-case the field-decl
+		   operand handling.  */
+		if (ix < i)
+		  lto_fixup_types (f2);
+		lto_streamer_cache_insert_at (cache, f1, ix);
+	      }
 
 	  /* If we found a tree that is equal to oldt replace it in the
 	     cache, so that further users (in the various LTO sections)
@@ -675,6 +638,22 @@ uniquify_nodes (struct data_in *data_in,
 	  lto_streamer_cache_insert_at (cache, t, i);
 	}
     }
+
+  /* Finally compute the canonical type of t.  From this point
+     there are no longer any types with TYPE_STRUCTURAL_EQUALITY_P
+     and its type-based alias problems.  This step requires the
+     TYPE_POINTER_TO lists being present, so make sure it is done
+     last.  */
+  for (i = len; i-- > from;)
+    {
+      tree t = VEC_index (tree, cache->nodes, i);
+      if (!t
+	  || !TYPE_P (t))
+	continue;
+
+      if (!TYPE_CANONICAL (t))
+	TYPE_CANONICAL (t) = gimple_register_canonical_type (t);
+    }
 }
 
 /* Read all the symbols from buffer DATA, using descriptors in DECL_DATA.
Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c	(revision 173900)
+++ gcc/gimple.c	(working copy)
@@ -4502,7 +4502,6 @@ gimple_register_type_1 (tree t, bool reg
 {
   void **slot;
   gimple_type_leader_entry *leader;
-  tree mv_leader;
 
   /* If we registered this type before return the cached result.  */
   leader = &gimple_type_leader[TYPE_UID (t) % GIMPLE_TYPE_LEADER_SIZE];
@@ -4521,91 +4520,23 @@ gimple_register_type_1 (tree t, bool reg
      case we do not care for the main variant leader.  */
   if (!registering_mv
       && TYPE_MAIN_VARIANT (t) != t)
-    mv_leader = gimple_register_type_1 (TYPE_MAIN_VARIANT (t), true);
-  else
-    mv_leader = t;
+    gimple_register_type_1 (TYPE_MAIN_VARIANT (t), true);
 
+  /* See if we already have an equivalent type registered.  */
   slot = htab_find_slot (gimple_types, t, INSERT);
   if (*slot
       && *(tree *)slot != t)
     {
       tree new_type = (tree) *((tree *) slot);
-
-      /* Do not merge types with different addressability.  */
-      gcc_assert (TREE_ADDRESSABLE (t) == TREE_ADDRESSABLE (new_type));
-
-      /* If t is not its main variant then make t unreachable from its
-	 main variant list.  Otherwise we'd queue up a lot of duplicates
-	 there.  */
-      if (t != TYPE_MAIN_VARIANT (t))
-	{
-	  tree tem = TYPE_MAIN_VARIANT (t);
-	  while (tem && TYPE_NEXT_VARIANT (tem) != t)
-	    tem = TYPE_NEXT_VARIANT (tem);
-	  if (tem)
-	    TYPE_NEXT_VARIANT (tem) = TYPE_NEXT_VARIANT (t);
-	  TYPE_NEXT_VARIANT (t) = NULL_TREE;
-	}
-
-      /* If we are a pointer then remove us from the pointer-to or
-	 reference-to chain.  Otherwise we'd queue up a lot of duplicates
-	 there.  */
-      if (TREE_CODE (t) == POINTER_TYPE)
-	{
-	  if (TYPE_POINTER_TO (TREE_TYPE (t)) == t)
-	    TYPE_POINTER_TO (TREE_TYPE (t)) = TYPE_NEXT_PTR_TO (t);
-	  else
-	    {
-	      tree tem = TYPE_POINTER_TO (TREE_TYPE (t));
-	      while (tem && TYPE_NEXT_PTR_TO (tem) != t)
-		tem = TYPE_NEXT_PTR_TO (tem);
-	      if (tem)
-		TYPE_NEXT_PTR_TO (tem) = TYPE_NEXT_PTR_TO (t);
-	    }
-	  TYPE_NEXT_PTR_TO (t) = NULL_TREE;
-	}
-      else if (TREE_CODE (t) == REFERENCE_TYPE)
-	{
-	  if (TYPE_REFERENCE_TO (TREE_TYPE (t)) == t)
-	    TYPE_REFERENCE_TO (TREE_TYPE (t)) = TYPE_NEXT_REF_TO (t);
-	  else
-	    {
-	      tree tem = TYPE_REFERENCE_TO (TREE_TYPE (t));
-	      while (tem && TYPE_NEXT_REF_TO (tem) != t)
-		tem = TYPE_NEXT_REF_TO (tem);
-	      if (tem)
-		TYPE_NEXT_REF_TO (tem) = TYPE_NEXT_REF_TO (t);
-	    }
-	  TYPE_NEXT_REF_TO (t) = NULL_TREE;
-	}
-
       leader->type = t;
       leader->leader = new_type;
-      t = new_type;
-    }
-  else
-    {
-      leader->type = t;
-      leader->leader = t;
-      /* We're the type leader.  Make our TYPE_MAIN_VARIANT valid.  */
-      if (TYPE_MAIN_VARIANT (t) != t
-	  && TYPE_MAIN_VARIANT (t) != mv_leader)
-	{
-	  /* Remove us from our main variant list as we are not the variant
-	     leader and the variant leader will change.  */
-	  tree tem = TYPE_MAIN_VARIANT (t);
-	  while (tem && TYPE_NEXT_VARIANT (tem) != t)
-	    tem = TYPE_NEXT_VARIANT (tem);
-	  if (tem)
-	    TYPE_NEXT_VARIANT (tem) = TYPE_NEXT_VARIANT (t);
-	  TYPE_NEXT_VARIANT (t) = NULL_TREE;
-	  /* Adjust our main variant.  Linking us into its variant list
-	     will happen at fixup time.  */
-	  TYPE_MAIN_VARIANT (t) = mv_leader;
-	}
-      *slot = (void *) t;
+      return new_type;
     }
 
+  /* If not, insert it to the cache and the hash.  */
+  leader->type = t;
+  leader->leader = t;
+  *slot = (void *) t;
   return t;
 }
 

Re: [PATCH][?/n] Cleanup LTO type merging

[H.J. Lu]

On Tue, May 17, 2011 at 6:03 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Tue, May 17, 2011 at 3:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, May 17, 2011 at 5:59 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, May 17, 2011 at 3:29 AM, Richard Guenther <rguenther@suse.de> wrote:
>>>> On Mon, 16 May 2011, H.J. Lu wrote:
>>>>
>>>>> On Mon, May 16, 2011 at 7:17 AM, Richard Guenther <rguenther@suse.de> wrote:
>>>>> >
>>>>> > The following patch improves hashing types by re-instantiating the
>>>>> > patch that makes us visit aggregate target types of pointers and
>>>>> > function return and argument types.  This halves the collision
>>>>> > rate on the type hash table for a linux-kernel build and improves
>>>>> > WPA compile-time from 3mins to 1mins and reduces memory usage by
>>>>> > 1GB for that testcase.
>>>>> >
>>>>> > Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
>>>>> > build-tested.
>>>>> >
>>>>> > Richard.
>>>>> >
>>>>> > (patch is reversed)
>>>>> >
>>>>> > 2011-05-16  Richard Guenther  <rguenther@suse.de>
>>>>> >
>>>>> >        * gimple.c (iterative_hash_gimple_type): Re-instantiate
>>>>> >        change to always visit pointer target and function result
>>>>> >        and argument types.
>>>>> >
>>>>>
>>>>> This caused:
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>>>>
>>>> I have reverted the patch for now.
>>>>
>>>
>>> It doesn't solve the problem and I reopened:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>>>
>>> Your followup patches may have similar issues.
>>>
>>
>> I think you reverted the WRONG patch:
>>
>> http://gcc.gnu.org/viewcvs?view=revision&revision=173827
>
> No, that was on purpose.
>

But it doesn't fix the problem.



-- 
H.J.

Re: [PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

On Tue, May 17, 2011 at 3:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, May 17, 2011 at 5:59 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, May 17, 2011 at 3:29 AM, Richard Guenther <rguenther@suse.de> wrote:
>>> On Mon, 16 May 2011, H.J. Lu wrote:
>>>
>>>> On Mon, May 16, 2011 at 7:17 AM, Richard Guenther <rguenther@suse.de> wrote:
>>>> >
>>>> > The following patch improves hashing types by re-instantiating the
>>>> > patch that makes us visit aggregate target types of pointers and
>>>> > function return and argument types.  This halves the collision
>>>> > rate on the type hash table for a linux-kernel build and improves
>>>> > WPA compile-time from 3mins to 1mins and reduces memory usage by
>>>> > 1GB for that testcase.
>>>> >
>>>> > Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
>>>> > build-tested.
>>>> >
>>>> > Richard.
>>>> >
>>>> > (patch is reversed)
>>>> >
>>>> > 2011-05-16  Richard Guenther  <rguenther@suse.de>
>>>> >
>>>> >        * gimple.c (iterative_hash_gimple_type): Re-instantiate
>>>> >        change to always visit pointer target and function result
>>>> >        and argument types.
>>>> >
>>>>
>>>> This caused:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>>>
>>> I have reverted the patch for now.
>>>
>>
>> It doesn't solve the problem and I reopened:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>>
>> Your followup patches may have similar issues.
>>
>
> I think you reverted the WRONG patch:
>
> http://gcc.gnu.org/viewcvs?view=revision&revision=173827

No, that was on purpose.

> --
> H.J.
>

Re: [PATCH][?/n] Cleanup LTO type merging

[H.J. Lu]

On Tue, May 17, 2011 at 5:59 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, May 17, 2011 at 3:29 AM, Richard Guenther <rguenther@suse.de> wrote:
>> On Mon, 16 May 2011, H.J. Lu wrote:
>>
>>> On Mon, May 16, 2011 at 7:17 AM, Richard Guenther <rguenther@suse.de> wrote:
>>> >
>>> > The following patch improves hashing types by re-instantiating the
>>> > patch that makes us visit aggregate target types of pointers and
>>> > function return and argument types.  This halves the collision
>>> > rate on the type hash table for a linux-kernel build and improves
>>> > WPA compile-time from 3mins to 1mins and reduces memory usage by
>>> > 1GB for that testcase.
>>> >
>>> > Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
>>> > build-tested.
>>> >
>>> > Richard.
>>> >
>>> > (patch is reversed)
>>> >
>>> > 2011-05-16  Richard Guenther  <rguenther@suse.de>
>>> >
>>> >        * gimple.c (iterative_hash_gimple_type): Re-instantiate
>>> >        change to always visit pointer target and function result
>>> >        and argument types.
>>> >
>>>
>>> This caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>>
>> I have reverted the patch for now.
>>
>
> It doesn't solve the problem and I reopened:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>
> Your followup patches may have similar issues.
>

I think you reverted the WRONG patch:

http://gcc.gnu.org/viewcvs?view=revision&revision=173827


-- 
H.J.

Re: [PATCH][?/n] Cleanup LTO type merging

[H.J. Lu]

On Tue, May 17, 2011 at 3:29 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Mon, 16 May 2011, H.J. Lu wrote:
>
>> On Mon, May 16, 2011 at 7:17 AM, Richard Guenther <rguenther@suse.de> wrote:
>> >
>> > The following patch improves hashing types by re-instantiating the
>> > patch that makes us visit aggregate target types of pointers and
>> > function return and argument types.  This halves the collision
>> > rate on the type hash table for a linux-kernel build and improves
>> > WPA compile-time from 3mins to 1mins and reduces memory usage by
>> > 1GB for that testcase.
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
>> > build-tested.
>> >
>> > Richard.
>> >
>> > (patch is reversed)
>> >
>> > 2011-05-16  Richard Guenther  <rguenther@suse.de>
>> >
>> >        * gimple.c (iterative_hash_gimple_type): Re-instantiate
>> >        change to always visit pointer target and function result
>> >        and argument types.
>> >
>>
>> This caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013
>
> I have reverted the patch for now.
>

It doesn't solve the problem and I reopened:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013

Your followup patches may have similar issues.

-- 
H.J.

Re: [PATCH][?/n] Cleanup LTO type merging

[Jan Hubicka]

> On Mon, 16 May 2011, Jan Hubicka wrote:
> 
> > > 
> > > I've seen us merge different named structs which happen to reside
> > > on the same variant list.  That's bogus, not only because we are
> > > adjusting TYPE_MAIN_VARIANT during incremental type-merging and
> > > fixup, so computing a persistent hash by looking at it looks
> > > fishy as well.
> > 
> > Hi,
> > as reported on IRC earlier, I get the segfault while building libxul
> > duea to infinite recursion problem.
> > 
> > I now however also get a lot more of the following ICEs:
> > In function '__unguarded_insertion_sort':
> > lto1: internal compiler error: in splice_child_die, at dwarf2out.c:8274
> > previously it reported once during Mozilla build (and I put testcase into
> > bugzilla), now it reproduces on many libraries. I did not see this problem
> > when applying only the SCC hasing change.
> 
> This change causes us to preserve more TYPE_DECLs I think, so we might
> run more often into pre-existing debuginfo issues.  Previously most
> of the types were merged into their nameless variant which probably
> didn't get output into debug info.
> 
> Do you by chance have small testcases for your problems? ;)

I think you might just look into one at http://gcc.gnu.org/bugzilla/show _bug.cgi?id=48354

Honza

Re: [PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 991 bytes --]

On Mon, 16 May 2011, H.J. Lu wrote:

> On Mon, May 16, 2011 at 7:17 AM, Richard Guenther <rguenther@suse.de> wrote:
> >
> > The following patch improves hashing types by re-instantiating the
> > patch that makes us visit aggregate target types of pointers and
> > function return and argument types.  This halves the collision
> > rate on the type hash table for a linux-kernel build and improves
> > WPA compile-time from 3mins to 1mins and reduces memory usage by
> > 1GB for that testcase.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
> > build-tested.
> >
> > Richard.
> >
> > (patch is reversed)
> >
> > 2011-05-16  Richard Guenther  <rguenther@suse.de>
> >
> >        * gimple.c (iterative_hash_gimple_type): Re-instantiate
> >        change to always visit pointer target and function result
> >        and argument types.
> >
> 
> This caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013

I have reverted the patch for now.

Richard.

Re: [PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3053 bytes --]

On Mon, 16 May 2011, Jan Hubicka wrote:

> > 
> > I've seen us merge different named structs which happen to reside
> > on the same variant list.  That's bogus, not only because we are
> > adjusting TYPE_MAIN_VARIANT during incremental type-merging and
> > fixup, so computing a persistent hash by looking at it looks
> > fishy as well.
> 
> Hi,
> as reported on IRC earlier, I get the segfault while building libxul
> duea to infinite recursion problem.
> 
> I now however also get a lot more of the following ICEs:
> In function '__unguarded_insertion_sort':
> lto1: internal compiler error: in splice_child_die, at dwarf2out.c:8274
> previously it reported once during Mozilla build (and I put testcase into
> bugzilla), now it reproduces on many libraries. I did not see this problem
> when applying only the SCC hasing change.

This change causes us to preserve more TYPE_DECLs I think, so we might
run more often into pre-existing debuginfo issues.  Previously most
of the types were merged into their nameless variant which probably
didn't get output into debug info.

Do you by chance have small testcases for your problems? ;)

Richard.

> perhaps another unrelated thing, but I now also get undefined symbols during
> the builds.
> 
> Honza
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > 
> > Richard.
> > 
> > 2011-05-16  Richard Guenther  <rguenther@suse.de>
> > 
> > 	* gimple.c (gimple_types_compatible_p_1): Use names of the
> > 	type itself, not its main variant.
> > 	(iterative_hash_gimple_type): Likewise.
> > 
> > Index: gcc/gimple.c
> > ===================================================================
> > *** gcc/gimple.c	(revision 173794)
> > --- gcc/gimple.c	(working copy)
> > *************** gimple_types_compatible_p_1 (tree t1, tr
> > *** 3817,3824 ****
> >   	tree f1, f2;
> >   
> >   	/* The struct tags shall compare equal.  */
> > ! 	if (!compare_type_names_p (TYPE_MAIN_VARIANT (t1),
> > ! 				   TYPE_MAIN_VARIANT (t2), false))
> >   	  goto different_types;
> >   
> >   	/* For aggregate types, all the fields must be the same.  */
> > --- 3817,3823 ----
> >   	tree f1, f2;
> >   
> >   	/* The struct tags shall compare equal.  */
> > ! 	if (!compare_type_names_p (t1, t2, false))
> >   	  goto different_types;
> >   
> >   	/* For aggregate types, all the fields must be the same.  */
> > *************** iterative_hash_gimple_type (tree type, h
> > *** 4193,4199 ****
> >         unsigned nf;
> >         tree f;
> >   
> > !       v = iterative_hash_name (TYPE_NAME (TYPE_MAIN_VARIANT (type)), v);
> >   
> >         for (f = TYPE_FIELDS (type), nf = 0; f; f = TREE_CHAIN (f))
> >   	{
> > --- 4192,4198 ----
> >         unsigned nf;
> >         tree f;
> >   
> > !       v = iterative_hash_name (TYPE_NAME (type), v);
> >   
> >         for (f = TYPE_FIELDS (type), nf = 0; f; f = TREE_CHAIN (f))
> >   	{
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH][?/n] Cleanup LTO type merging

[H.J. Lu]

On Mon, May 16, 2011 at 7:17 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> The following patch improves hashing types by re-instantiating the
> patch that makes us visit aggregate target types of pointers and
> function return and argument types.  This halves the collision
> rate on the type hash table for a linux-kernel build and improves
> WPA compile-time from 3mins to 1mins and reduces memory usage by
> 1GB for that testcase.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
> build-tested.
>
> Richard.
>
> (patch is reversed)
>
> 2011-05-16  Richard Guenther  <rguenther@suse.de>
>
>        * gimple.c (iterative_hash_gimple_type): Re-instantiate
>        change to always visit pointer target and function result
>        and argument types.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013


H.J.

Re: [PATCH][?/n] Cleanup LTO type merging

[H.J. Lu]

On Mon, May 16, 2011 at 11:32 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> I've seen us merge different named structs which happen to reside
>> on the same variant list.  That's bogus, not only because we are
>> adjusting TYPE_MAIN_VARIANT during incremental type-merging and
>> fixup, so computing a persistent hash by looking at it looks
>> fishy as well.
>
> Hi,
> as reported on IRC earlier, I get the segfault while building libxul
> duea to infinite recursion problem.
>
> I now however also get a lot more of the following ICEs:
> In function '__unguarded_insertion_sort':
> lto1: internal compiler error: in splice_child_die, at dwarf2out.c:8274
> previously it reported once during Mozilla build (and I put testcase into
> bugzilla), now it reproduces on many libraries. I did not see this problem
> when applying only the SCC hasing change.

That is

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49013


-- 
H.J.

Re: [PATCH][?/n] Cleanup LTO type merging

[Jan Hubicka]

> 
> I've seen us merge different named structs which happen to reside
> on the same variant list.  That's bogus, not only because we are
> adjusting TYPE_MAIN_VARIANT during incremental type-merging and
> fixup, so computing a persistent hash by looking at it looks
> fishy as well.

Hi,
as reported on IRC earlier, I get the segfault while building libxul
duea to infinite recursion problem.

I now however also get a lot more of the following ICEs:
In function '__unguarded_insertion_sort':
lto1: internal compiler error: in splice_child_die, at dwarf2out.c:8274
previously it reported once during Mozilla build (and I put testcase into
bugzilla), now it reproduces on many libraries. I did not see this problem
when applying only the SCC hasing change.

perhaps another unrelated thing, but I now also get undefined symbols during
the builds.

Honza
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Richard.
> 
> 2011-05-16  Richard Guenther  <rguenther@suse.de>
> 
> 	* gimple.c (gimple_types_compatible_p_1): Use names of the
> 	type itself, not its main variant.
> 	(iterative_hash_gimple_type): Likewise.
> 
> Index: gcc/gimple.c
> ===================================================================
> *** gcc/gimple.c	(revision 173794)
> --- gcc/gimple.c	(working copy)
> *************** gimple_types_compatible_p_1 (tree t1, tr
> *** 3817,3824 ****
>   	tree f1, f2;
>   
>   	/* The struct tags shall compare equal.  */
> ! 	if (!compare_type_names_p (TYPE_MAIN_VARIANT (t1),
> ! 				   TYPE_MAIN_VARIANT (t2), false))
>   	  goto different_types;
>   
>   	/* For aggregate types, all the fields must be the same.  */
> --- 3817,3823 ----
>   	tree f1, f2;
>   
>   	/* The struct tags shall compare equal.  */
> ! 	if (!compare_type_names_p (t1, t2, false))
>   	  goto different_types;
>   
>   	/* For aggregate types, all the fields must be the same.  */
> *************** iterative_hash_gimple_type (tree type, h
> *** 4193,4199 ****
>         unsigned nf;
>         tree f;
>   
> !       v = iterative_hash_name (TYPE_NAME (TYPE_MAIN_VARIANT (type)), v);
>   
>         for (f = TYPE_FIELDS (type), nf = 0; f; f = TREE_CHAIN (f))
>   	{
> --- 4192,4198 ----
>         unsigned nf;
>         tree f;
>   
> !       v = iterative_hash_name (TYPE_NAME (type), v);
>   
>         for (f = TYPE_FIELDS (type), nf = 0; f; f = TREE_CHAIN (f))
>   	{

[PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

I've seen us merge different named structs which happen to reside
on the same variant list.  That's bogus, not only because we are
adjusting TYPE_MAIN_VARIANT during incremental type-merging and
fixup, so computing a persistent hash by looking at it looks
fishy as well.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2011-05-16  Richard Guenther  <rguenther@suse.de>

	* gimple.c (gimple_types_compatible_p_1): Use names of the
	type itself, not its main variant.
	(iterative_hash_gimple_type): Likewise.

Index: gcc/gimple.c
===================================================================
*** gcc/gimple.c	(revision 173794)
--- gcc/gimple.c	(working copy)
*************** gimple_types_compatible_p_1 (tree t1, tr
*** 3817,3824 ****
  	tree f1, f2;
  
  	/* The struct tags shall compare equal.  */
! 	if (!compare_type_names_p (TYPE_MAIN_VARIANT (t1),
! 				   TYPE_MAIN_VARIANT (t2), false))
  	  goto different_types;
  
  	/* For aggregate types, all the fields must be the same.  */
--- 3817,3823 ----
  	tree f1, f2;
  
  	/* The struct tags shall compare equal.  */
! 	if (!compare_type_names_p (t1, t2, false))
  	  goto different_types;
  
  	/* For aggregate types, all the fields must be the same.  */
*************** iterative_hash_gimple_type (tree type, h
*** 4193,4199 ****
        unsigned nf;
        tree f;
  
!       v = iterative_hash_name (TYPE_NAME (TYPE_MAIN_VARIANT (type)), v);
  
        for (f = TYPE_FIELDS (type), nf = 0; f; f = TREE_CHAIN (f))
  	{
--- 4192,4198 ----
        unsigned nf;
        tree f;
  
!       v = iterative_hash_name (TYPE_NAME (type), v);
  
        for (f = TYPE_FIELDS (type), nf = 0; f; f = TREE_CHAIN (f))
  	{

[PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

The following patch improves hashing types by re-instantiating the
patch that makes us visit aggregate target types of pointers and
function return and argument types.  This halves the collision
rate on the type hash table for a linux-kernel build and improves
WPA compile-time from 3mins to 1mins and reduces memory usage by
1GB for that testcase.

Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC2k6
build-tested.

Richard.

(patch is reversed)

2011-05-16  Richard Guenther  <rguenther@suse.de>

	* gimple.c (iterative_hash_gimple_type): Re-instantiate
	change to always visit pointer target and function result
	and argument types.

Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c	(revision 173725)
+++ gcc/gimple.c	(working copy)
@@ -4110,10 +4110,20 @@ iterative_hash_gimple_type (tree type, h
     }
 
   /* For pointer and reference types, fold in information about the type
-     pointed to.  */
+     pointed to but do not recurse into possibly incomplete types to
+     avoid hash differences for complete vs. incomplete types.  */
   if (POINTER_TYPE_P (type))
-    v = visit (TREE_TYPE (type), state, v,
-	       sccstack, sccstate, sccstate_obstack);
+    {
+      if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (type)))
+	{
+	  v = iterative_hash_hashval_t (TREE_CODE (TREE_TYPE (type)), v);
+	  v = iterative_hash_name
+		(TYPE_NAME (TYPE_MAIN_VARIANT (TREE_TYPE (type))), v);
+	}
+      else
+	v = visit (TREE_TYPE (type), state, v,
+		   sccstack, sccstate, sccstate_obstack);
+    }
 
   /* For integer types hash the types min/max values and the string flag.  */
   if (TREE_CODE (type) == INTEGER_TYPE)
@@ -4154,13 +4164,29 @@ iterative_hash_gimple_type (tree type, h
 	v = visit (TYPE_METHOD_BASETYPE (type), state, v,
 		   sccstack, sccstate, sccstate_obstack);
 
-      /* Check result and argument types.  */
-      v = visit (TREE_TYPE (type), state, v,
-		 sccstack, sccstate, sccstate_obstack);
+      /* For result types allow mismatch in completeness.  */
+      if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (type)))
+	{
+	  v = iterative_hash_hashval_t (TREE_CODE (TREE_TYPE (type)), v);
+	  v = iterative_hash_name
+		(TYPE_NAME (TYPE_MAIN_VARIANT (TREE_TYPE (type))), v);
+	}
+      else
+	v = visit (TREE_TYPE (type), state, v,
+		   sccstack, sccstate, sccstate_obstack);
+
       for (p = TYPE_ARG_TYPES (type), na = 0; p; p = TREE_CHAIN (p))
 	{
-	  v = visit (TREE_VALUE (p), state, v,
-		     sccstack, sccstate, sccstate_obstack);
+	  /* For argument types allow mismatch in completeness.  */
+	  if (RECORD_OR_UNION_TYPE_P (TREE_VALUE (p)))
+	    {
+	      v = iterative_hash_hashval_t (TREE_CODE (TREE_VALUE (p)), v);
+	      v = iterative_hash_name
+		    (TYPE_NAME (TYPE_MAIN_VARIANT (TREE_VALUE (p))), v);
+	    }
+	  else
+	    v = visit (TREE_VALUE (p), state, v,
+		       sccstack, sccstate, sccstate_obstack);
 	  na++;
 	}
 

[PATCH][?/n] Cleanup LTO type merging

[Richard Guenther]

This gets rid of type pair caching in the canonical type comparison.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2011-05-13  Richard Guenther  <rguenther@suse.de>

	* gimple.c (gimple_canonical_types_compatible_p): Do not use
	type-pair caching, do not compare hashes.

Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c	(revision 173730)
+++ gcc/gimple.c	(working copy)
@@ -4569,8 +4569,6 @@ gimple_register_type (tree t)
 static bool
 gimple_canonical_types_compatible_p (tree t1, tree t2)
 {
-  type_pair_t p = NULL;
-
   /* Before starting to set up the SCC machinery handle simple cases.  */
 
   /* Check first for the obvious case of pointer identity.  */
@@ -4656,27 +4654,9 @@ gimple_canonical_types_compatible_p (tre
       return true;
     }
 
-  /* If the hash values of t1 and t2 are different the types can't
-     possibly be the same.  This helps keeping the type-pair hashtable
-     small, only tracking comparisons for hash collisions.  */
-  if (gimple_canonical_type_hash (t1) != gimple_canonical_type_hash (t2))
-    return false;
-
-  /* If we've visited this type pair before (in the case of aggregates
-     with self-referential types), and we made a decision, return it.  */
-  p = lookup_type_pair (t1, t2, &gtc_visited, &gtc_ob);
-  if (p->same_p[GTC_DIAG] == 0 || p->same_p[GTC_DIAG] == 1)
-    {
-      /* We have already decided whether T1 and T2 are the
-	 same, return the cached result.  */
-      return p->same_p[GTC_DIAG] == 1;
-    }
-
-  gcc_assert (p->same_p[GTC_DIAG] == -2);
-
   /* If their attributes are not the same they can't be the same type.  */
   if (!attribute_list_equal (TYPE_ATTRIBUTES (t1), TYPE_ATTRIBUTES (t2)))
-    goto different_types;
+    return false;
 
   /* Do type-specific comparisons.  */
   switch (TREE_CODE (t1))
@@ -4687,7 +4667,7 @@ gimple_canonical_types_compatible_p (tre
       if (!gimple_canonical_types_compatible_p (TREE_TYPE (t1), TREE_TYPE (t2))
 	  || TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2)
 	  || TYPE_NONALIASED_COMPONENT (t1) != TYPE_NONALIASED_COMPONENT (t2))
-	goto different_types;
+	return false;
       else
 	{
 	  tree i1 = TYPE_DOMAIN (t1);
@@ -4696,16 +4676,16 @@ gimple_canonical_types_compatible_p (tre
 	  /* For an incomplete external array, the type domain can be
  	     NULL_TREE.  Check this condition also.  */
 	  if (i1 == NULL_TREE && i2 == NULL_TREE)
-	    goto same_types;
+	    return true;
 	  else if (i1 == NULL_TREE || i2 == NULL_TREE)
-	    goto different_types;
+	    return false;
 	  /* If for a complete array type the possibly gimplified sizes
 	     are different the types are different.  */
 	  else if (((TYPE_SIZE (i1) != NULL) ^ (TYPE_SIZE (i2) != NULL))
 		   || (TYPE_SIZE (i1)
 		       && TYPE_SIZE (i2)
 		       && !operand_equal_p (TYPE_SIZE (i1), TYPE_SIZE (i2), 0)))
-	    goto different_types;
+	    return false;
 	  else
 	    {
 	      tree min1 = TYPE_MIN_VALUE (i1);
@@ -4724,9 +4704,9 @@ gimple_canonical_types_compatible_p (tre
 			  && ((TREE_CODE (max1) == PLACEHOLDER_EXPR
 			       && TREE_CODE (max2) == PLACEHOLDER_EXPR)
 			      || operand_equal_p (max1, max2, 0)))))
-		goto same_types;
+		return true;
 	      else
-		goto different_types;
+		return false;
 	    }
 	}
 
@@ -4734,7 +4714,7 @@ gimple_canonical_types_compatible_p (tre
       /* Method types should belong to the same class.  */
       if (!gimple_canonical_types_compatible_p
 	     (TYPE_METHOD_BASETYPE (t1), TYPE_METHOD_BASETYPE (t2)))
-	goto different_types;
+	return false;
 
       /* Fallthru  */
 
@@ -4745,13 +4725,13 @@ gimple_canonical_types_compatible_p (tre
 	     (TREE_TYPE (t1), TREE_TYPE (t2))
 	  && !gimple_canonical_types_compatible_p
 	        (TREE_TYPE (t1), TREE_TYPE (t2)))
-	goto different_types;
+	return false;
 
       if (!comp_type_attributes (t1, t2))
-	goto different_types;
+	return false;
 
       if (TYPE_ARG_TYPES (t1) == TYPE_ARG_TYPES (t2))
-	goto same_types;
+	return true;
       else
 	{
 	  tree parms1, parms2;
@@ -4764,13 +4744,13 @@ gimple_canonical_types_compatible_p (tre
 		         (TREE_VALUE (parms1), TREE_VALUE (parms2))
 		  && !gimple_canonical_types_compatible_p
 		        (TREE_VALUE (parms1), TREE_VALUE (parms2)))
-		goto different_types;
+		return false;
 	    }
 
 	  if (parms1 || parms2)
-	    goto different_types;
+	    return false;
 
-	  goto same_types;
+	  return true;
 	}
 
     case RECORD_TYPE:
@@ -4789,30 +4769,20 @@ gimple_canonical_types_compatible_p (tre
 		|| !gimple_compare_field_offset (f1, f2)
 		|| !gimple_canonical_types_compatible_p
 		      (TREE_TYPE (f1), TREE_TYPE (f2)))
-	      goto different_types;
+	      return false;
 	  }
 
 	/* If one aggregate has more fields than the other, they
 	   are not the same.  */
 	if (f1 || f2)
-	  goto different_types;
+	  return false;
 
-	goto same_types;
+	return true;
       }
 
     default:
       gcc_unreachable ();
     }
-
-  /* Common exit path for types that are not compatible.  */
-different_types:
-  p->same_p[GTC_DIAG] = 0;
-  return false;
-
-  /* Common exit path for types that are compatible.  */
-same_types:
-  p->same_p[GTC_DIAG] = 1;
-  return true;
 }