diff mbox

[48/50] varasm.c:const_rtx_hash

Message ID 877g2p63ds.fsf@googlemail.com
State New
Headers show

Commit Message

Richard Sandiford Aug. 3, 2014, 2:42 p.m. UTC 
const_rtx_hash_1 had code to hash all elements of a CONST_VECTOR,
but const_rtx_hash also hashes all subrtxes, so we'd end up hashing
the same thing twice.  This looked unintentional so I just removed the
CONST_VECTOR case.  If instead it was a deliberate decision then I think
it deserves a comment.


gcc/
	* varasm.c: Include rtl-iter.h.
	(const_rtx_hash_1): Take a const_rtx rather than an rtx *.
	Remove the pointer to the cumulative hashval_t and just return
	the hash for this rtx instead.  Remove recursive CONST_VECTOR case.
	(const_rtx_hash): Use FOR_EACH_SUBRTX instead of for_each_rtx.
	Accumulate the hashval_ts here instead of const_rtx_hash_1.

Comments

Jeff Law Aug. 5, 2014, 10:18 p.m. UTC | #1 
On 08/03/14 08:42, Richard Sandiford wrote:
> const_rtx_hash_1 had code to hash all elements of a CONST_VECTOR,
> but const_rtx_hash also hashes all subrtxes, so we'd end up hashing
> the same thing twice.  This looked unintentional so I just removed the
> CONST_VECTOR case.  If instead it was a deliberate decision then I think
> it deserves a comment.
>
>
> gcc/
> 	* varasm.c: Include rtl-iter.h.
> 	(const_rtx_hash_1): Take a const_rtx rather than an rtx *.
> 	Remove the pointer to the cumulative hashval_t and just return
> 	the hash for this rtx instead.  Remove recursive CONST_VECTOR case.
> 	(const_rtx_hash): Use FOR_EACH_SUBRTX instead of for_each_rtx.
> 	Accumulate the hashval_ts here instead of const_rtx_hash_1.
This is fine.  Though I wonder if it's going to conflict with Andi's 
work on inchash.  If so, consider adjustments to handle Andi's changes 
pre-approved.  Just post the final patch for archival purposes if you 
need to change it.

Thanks,
jeff
diff mbox

Patch

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2014-08-03 11:25:30.450157225 +0100
+++ gcc/varasm.c	2014-08-03 11:25:33.667189030 +0100
@@ -54,6 +54,7 @@  Software Foundation; either version 3, o
 #include "hash-set.h"
 #include "asan.h"
 #include "basic-block.h"
+#include "rtl-iter.h"
 
 #ifdef XCOFF_DEBUGGING_INFO
 #include "xcoffout.h"		/* Needed for external data
@@ -3481,19 +3482,17 @@  const_desc_rtx_eq (const void *a, const
   return rtx_equal_p (x->constant, y->constant);
 }
 
-/* This is the worker function for const_rtx_hash, called via for_each_rtx.  */
+/* Hash one component of a constant.  */
 
-static int
-const_rtx_hash_1 (rtx *xp, void *data)
+static hashval_t
+const_rtx_hash_1 (const_rtx x)
 {
   unsigned HOST_WIDE_INT hwi;
   enum machine_mode mode;
   enum rtx_code code;
-  hashval_t h, *hp;
-  rtx x;
+  hashval_t h;
   int i;
 
-  x = *xp;
   code = GET_CODE (x);
   mode = GET_MODE (x);
   h = (hashval_t) code * 1048573 + mode;
@@ -3539,14 +3538,6 @@  const_rtx_hash_1 (rtx *xp, void *data)
       h ^= fixed_hash (CONST_FIXED_VALUE (x));
       break;
 
-    case CONST_VECTOR:
-      {
-	int i;
-	for (i = XVECLEN (x, 0); i-- > 0; )
-	  h = h * 251 + const_rtx_hash_1 (&XVECEXP (x, 0, i), data);
-      }
-      break;
-
     case SYMBOL_REF:
       h ^= htab_hash_string (XSTR (x, 0));
       break;
@@ -3564,9 +3555,7 @@  const_rtx_hash_1 (rtx *xp, void *data)
       break;
     }
 
-  hp = (hashval_t *) data;
-  *hp = *hp * 509 + h;
-  return 0;
+  return h;
 }
 
 /* Compute a hash value for X, which should be a constant.  */
@@ -3575,7 +3564,9 @@  const_rtx_hash_1 (rtx *xp, void *data)
 const_rtx_hash (rtx x)
 {
   hashval_t h = 0;
-  for_each_rtx (&x, const_rtx_hash_1, &h);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, ALL)
+    h = h * 509 + const_rtx_hash_1 (*iter);
   return h;
 }