diff mbox

Fix array type merging in LTO mode

Message ID 201205201956.50154.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou May 20, 2012, 5:56 p.m. UTC 
Hi,

since http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00833.html, canonical type 
merging for arrays takes hours instead of minutes for big Ada applications.
The problem is that iterative_hash_canonical_type doesn't hash TYPE_MIN_VALUE 
and TYPE_MAX_VALUE for integer types anymore, so TYPE_DOMAIN is effectively 
not hashed anymore and the number of collisions goes to the roof in Ada.

Fixed by the attached patch, which also removes a bogus comparison of the 
TYPE_SIZE of TYPE_DOMAIN in gimple_[canonical]types_compatible_p.  LTO 
bootstrapped on x86_64-suse-linux, OK for mainline and 4.7 branch?


2012-05-20  Eric Botcazou  <ebotcazou@adacore.com>

	* gimple.c (gimple_types_compatible_p_1) <ARRAY_TYPE>: Remove bogus
	size handling.
	(gimple_canonical_types_compatible_p) <ARRAY_TYPE>: Likewise.
	(iterative_hash_gimple_type): Adjust comment.
	(iterative_hash_canonical_type): Likewise.  Hash the bounds of the
	domain for an array type.

Comments

Richard Biener May 21, 2012, 10:13 a.m. UTC | #1 
On Sun, May 20, 2012 at 7:56 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> since http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00833.html, canonical type
> merging for arrays takes hours instead of minutes for big Ada applications.
> The problem is that iterative_hash_canonical_type doesn't hash TYPE_MIN_VALUE
> and TYPE_MAX_VALUE for integer types anymore, so TYPE_DOMAIN is effectively
> not hashed anymore and the number of collisions goes to the roof in Ada.
>
> Fixed by the attached patch, which also removes a bogus comparison of the
> TYPE_SIZE of TYPE_DOMAIN in gimple_[canonical]types_compatible_p.  LTO
> bootstrapped on x86_64-suse-linux, OK for mainline and 4.7 branch?

-  /* For integer types hash the types min/max values and the string flag.  */
+  /* For integer types hash the sizetype flag and the string flag.  */
   if (TREE_CODE (type) == INTEGER_TYPE)
     v = iterative_hash_hashval_t (TYPE_STRING_FLAG (type), v);

The sizetype flag is no longer present (the patch will differ for the branch).

+  /* For array types hash the domain and its bounds, and the string flag.  */
+  if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
     {
       v = iterative_hash_hashval_t (TYPE_STRING_FLAG (type), v);
       v = iterative_hash_canonical_type (TYPE_DOMAIN (type), v);
+      /* OMP lowering can introduce error_mark_node in place of
+        random local decls in types.  */
+      if (TYPE_MIN_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
+       v = iterative_hash_expr (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), v);
+      if (TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
+       v = iterative_hash_expr (TYPE_MAX_VALUE (TYPE_DOMAIN (type)), v);
     }

I suppose hashing TYPE_DOMAIN itself is then useless?  Practically
it will be always sizetype anyway ... and if we start to see different
domain types arrays with the same actual domain [0, 1] should be
still merged for canonical merging.

Ok with the comment adjustment and not hashing TYPE_DOMAIN itself.

Thanks,
Richard.


>
> 2012-05-20  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * gimple.c (gimple_types_compatible_p_1) <ARRAY_TYPE>: Remove bogus
>        size handling.
>        (gimple_canonical_types_compatible_p) <ARRAY_TYPE>: Likewise.
>        (iterative_hash_gimple_type): Adjust comment.
>        (iterative_hash_canonical_type): Likewise.  Hash the bounds of the
>        domain for an array type.
>
> --
> Eric Botcazou
diff mbox

Patch

Index: gimple.c
===================================================================
--- gimple.c	(revision 187680)
+++ gimple.c	(working copy)
@@ -3445,13 +3445,6 @@  gimple_types_compatible_p_1 (tree t1, tr
 	    goto same_types;
 	  else if (i1 == NULL_TREE || i2 == NULL_TREE)
 	    goto different_types;
-	  /* 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;
 	  else
 	    {
 	      tree min1 = TYPE_MIN_VALUE (i1);
@@ -3962,9 +3955,8 @@  iterative_hash_gimple_type (tree type, h
       v = iterative_hash_hashval_t (TYPE_STRING_FLAG (type), v);
     }
 
-  /* For array types hash their domain and the string flag.  */
-  if (TREE_CODE (type) == ARRAY_TYPE
-      && TYPE_DOMAIN (type))
+  /* For array types hash the domain and the string flag.  */
+  if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
     {
       v = iterative_hash_hashval_t (TYPE_STRING_FLAG (type), v);
       v = visit (TYPE_DOMAIN (type), state, v,
@@ -4191,16 +4183,21 @@  iterative_hash_canonical_type (tree type
       v = iterative_hash_hashval_t (TREE_CODE (TREE_TYPE (type)), v);
     }
 
-  /* For integer types hash the types min/max values and the string flag.  */
+  /* For integer types hash the sizetype flag and the string flag.  */
   if (TREE_CODE (type) == INTEGER_TYPE)
     v = iterative_hash_hashval_t (TYPE_STRING_FLAG (type), v);
 
-  /* For array types hash their domain and the string flag.  */
-  if (TREE_CODE (type) == ARRAY_TYPE
-      && TYPE_DOMAIN (type))
+  /* For array types hash the domain and its bounds, and the string flag.  */
+  if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
     {
       v = iterative_hash_hashval_t (TYPE_STRING_FLAG (type), v);
       v = iterative_hash_canonical_type (TYPE_DOMAIN (type), v);
+      /* OMP lowering can introduce error_mark_node in place of
+	 random local decls in types.  */
+      if (TYPE_MIN_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
+	v = iterative_hash_expr (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), v);
+      if (TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
+	v = iterative_hash_expr (TYPE_MAX_VALUE (TYPE_DOMAIN (type)), v);
     }
 
   /* Recurse for aggregates with a single element type.  */
@@ -4468,13 +4465,6 @@  gimple_canonical_types_compatible_p (tre
 	    return true;
 	  else if (i1 == NULL_TREE || i2 == NULL_TREE)
 	    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)))
-	    return false;
 	  else
 	    {
 	      tree min1 = TYPE_MIN_VALUE (i1);