[tuples] Fix bootstrap on 64 bit hosts

[tuples] Fix bootstrap on 64 bit hosts

[Diego Novillo]

[-- Attachment #1: Type: text/plain, Size: 129 bytes --]

tree-vectorizer.c:hash_gimple_stmt was casting 64bit pointers to 32bit
ints.  Fixed by using htab_hash_pointer instead.


Diego.

[-- Attachment #2: 20080625-fix-ptr-cast.diff.txt --]
[-- Type: text/plain, Size: 1246 bytes --]

2008-06-25  Diego Novillo  <dnovillo@google.com>

	* tree-vectorizer.c (hash_gimple_stmt): Remove.
	(eq_gimple_stmt): Remove.
	(init_stmt_vec_info_htab): Use htab_hash_pointer and
	htab_eq_pointer for STMT_VEC_INFO_HTAB.

Index: tree-vectorizer.c
===================================================================
--- tree-vectorizer.c	(revision 137105)
+++ tree-vectorizer.c	(working copy)
@@ -1566,31 +1566,14 @@ new_stmt_vec_info (gimple stmt, loop_vec
   return res;
 }
 
-/* Hash function for stmt_vec_info table. P is the addres of a GIMPLE. */
-
-static hashval_t
-hash_gimple_stmt (const void *p)
-{
-  return (hashval_t) p;
-}
-
-/* Equality function for stmt_vec_info table key. P1 and P2 are address of
-   two GIMPLE statements.  They are equal if and only if they point the
-   same statement. */
 
-static int
-eq_gimple_stmt (const void *p1, const void *p2)
-{
-  return p1 == p2;
-}
-
-/* Createw hash table for stmt_vec_info. */
+/* Create a hash table for stmt_vec_info. */
 
 void
 init_stmt_vec_info_htab (void)
 {
   gcc_assert (!stmt_vec_info_htab);
-  stmt_vec_info_htab = htab_create (10, hash_gimple_stmt, eq_gimple_stmt,
+  stmt_vec_info_htab = htab_create (10, htab_hash_pointer, htab_eq_pointer,
 				    NULL);
 }
 

Re: [tuples] Fix bootstrap on 64 bit hosts

[Jakub Jelinek]

On Wed, Jun 25, 2008 at 09:21:19AM -0400, Diego Novillo wrote:
> tree-vectorizer.c:hash_gimple_stmt was casting 64bit pointers to 32bit
> ints.  Fixed by using htab_hash_pointer instead.

> 2008-06-25  Diego Novillo  <dnovillo@google.com>
> 
> 	* tree-vectorizer.c (hash_gimple_stmt): Remove.
> 	(eq_gimple_stmt): Remove.
> 	(init_stmt_vec_info_htab): Use htab_hash_pointer and
> 	htab_eq_pointer for STMT_VEC_INFO_HTAB.

This looks wrong.  gimple will never be equal to stmt_vec_info, so this
htab acts as a write-only hashtab - every lookup will return a new slot,
as htab_eq_pointer will always return false.
Either we can use a pointer_map, or, given that we have
STMT_VINFO_STMT in stmt_vec_info, we can do following.  Note even
storing NULL info the htable slot was incorrect.

2008-07-01  Jakub Jelinek  <jakub@redhat.com>

	* tree-vectorizer.h (vinfo_for_stmt): Use htab_find_slot_with_hash.
	(set_vinfo_for_stmt): Likewise.  If info is NULL, delete entry from
	hash table.
	* tree-vectorizer.c (stmt_vec_info_eq, stmt_vec_info_hash): New
	functions.
	(init_stmt_vec_info_htab): Use them instead of htab_hash_pointer
	and htab_eq_pointer.

--- gcc/tree-vectorizer.c.jj	2008-06-27 18:54:54.000000000 +0200
+++ gcc/tree-vectorizer.c	2008-07-01 11:54:34.000000000 +0200
@@ -1566,6 +1566,19 @@ new_stmt_vec_info (gimple stmt, loop_vec
   return res;
 }
 
+static int
+stmt_vec_info_eq (const void *p1, const void *p2)
+{
+  return (const_gimple) STMT_VINFO_STMT ((const struct _stmt_vec_info *) p1)
+	 == (const_gimple) p2;
+}
+
+static hashval_t
+stmt_vec_info_hash (const void *p)
+{
+  const struct _stmt_vec_info *i = (const struct _stmt_vec_info *) p;
+  return htab_hash_pointer (STMT_VINFO_STMT (i));
+}
 
 /* Create a hash table for stmt_vec_info. */
 
@@ -1573,7 +1586,7 @@ void
 init_stmt_vec_info_htab (void)
 {
   gcc_assert (!stmt_vec_info_htab);
-  stmt_vec_info_htab = htab_create (10, htab_hash_pointer, htab_eq_pointer,
+  stmt_vec_info_htab = htab_create (10, stmt_vec_info_hash, stmt_vec_info_eq,
 				    NULL);
 }
 
--- gcc/tree-vectorizer.h.jj	2008-06-27 18:54:54.000000000 +0200
+++ gcc/tree-vectorizer.h	2008-07-01 12:06:42.000000000 +0200
@@ -531,7 +531,8 @@ static inline stmt_vec_info
 vinfo_for_stmt (gimple stmt)
 {
   void **slot;
-  slot = htab_find_slot (stmt_vec_info_htab, stmt, NO_INSERT);
+  slot = htab_find_slot_with_hash (stmt_vec_info_htab, stmt,
+				   htab_hash_pointer (stmt), NO_INSERT);
   return slot ? (stmt_vec_info) *slot : NULL;
 }
 
@@ -539,9 +540,20 @@ static inline void
 set_vinfo_for_stmt (gimple stmt, stmt_vec_info info)
 {
   void **slot;
-  slot = htab_find_slot (stmt_vec_info_htab, stmt, INSERT);
-  gcc_assert (!(*slot && info));
-  *slot = info;
+  if (info)
+    {
+      slot = htab_find_slot_with_hash (stmt_vec_info_htab, stmt,
+				       htab_hash_pointer (stmt), INSERT);
+      gcc_assert (!*slot);
+      *slot = info;
+    }
+  else
+    {
+      slot = htab_find_slot_with_hash (stmt_vec_info_htab, stmt,
+				       htab_hash_pointer (stmt), NO_INSERT);
+      if (slot)
+	htab_clear_slot (stmt_vec_info_htab, slot);
+    }
 }
 
 static inline bool

	Jakub

[tuples] Fix {set_,}vinfo_for_stmt (take 2)

[Jakub Jelinek]

On Tue, Jul 01, 2008 at 06:12:29AM -0400, Jakub Jelinek wrote:
> On Wed, Jun 25, 2008 at 09:21:19AM -0400, Diego Novillo wrote:
> > tree-vectorizer.c:hash_gimple_stmt was casting 64bit pointers to 32bit
> > ints.  Fixed by using htab_hash_pointer instead.
> 
> > 2008-06-25  Diego Novillo  <dnovillo@google.com>
> > 
> > 	* tree-vectorizer.c (hash_gimple_stmt): Remove.
> > 	(eq_gimple_stmt): Remove.
> > 	(init_stmt_vec_info_htab): Use htab_hash_pointer and
> > 	htab_eq_pointer for STMT_VEC_INFO_HTAB.
> 
> This looks wrong.  gimple will never be equal to stmt_vec_info, so this
> htab acts as a write-only hashtab - every lookup will return a new slot,
> as htab_eq_pointer will always return false.
> Either we can use a pointer_map, or, given that we have
> STMT_VINFO_STMT in stmt_vec_info, we can do following.  Note even
> storing NULL info the htable slot was incorrect.

During testing I've discovered one issue with that, free_stmt_vec_info
first freed the info and only afterwards deregistered it from the hash
table.  Now that the hash table eq/hash functions look at
STMT_VINFO_STMT of infos contained in the hash tables this is fatal as
free might clobber that memory.

2008-07-01  Jakub Jelinek  <jakub@redhat.com>

	* tree-vectorizer.h (vinfo_for_stmt): Use htab_find_slot_with_hash.
	(set_vinfo_for_stmt): Likewise.  If info is NULL, delete entry from
	hash table.
	* tree-vectorizer.c (stmt_vec_info_eq, stmt_vec_info_hash): New
	functions.
	(init_stmt_vec_info_htab): Use them instead of htab_hash_pointer
	and htab_eq_pointer.
	(free_stmt_vec_info): Free stmt_info only after set_vinfo_for_stmt
	call.

--- gcc/tree-vectorizer.c.jj	2008-06-27 18:54:54.000000000 +0200
+++ gcc/tree-vectorizer.c	2008-07-01 15:00:42.000000000 +0200
@@ -1566,6 +1566,19 @@ new_stmt_vec_info (gimple stmt, loop_vec
   return res;
 }
 
+static int
+stmt_vec_info_eq (const void *p1, const void *p2)
+{
+  return (const_gimple) STMT_VINFO_STMT ((const struct _stmt_vec_info *) p1)
+	 == (const_gimple) p2;
+}
+
+static hashval_t
+stmt_vec_info_hash (const void *p)
+{
+  const struct _stmt_vec_info *i = (const struct _stmt_vec_info *) p;
+  return htab_hash_pointer (STMT_VINFO_STMT (i));
+}
 
 /* Create a hash table for stmt_vec_info. */
 
@@ -1573,7 +1586,7 @@ void
 init_stmt_vec_info_htab (void)
 {
   gcc_assert (!stmt_vec_info_htab);
-  stmt_vec_info_htab = htab_create (10, htab_hash_pointer, htab_eq_pointer,
+  stmt_vec_info_htab = htab_create (10, stmt_vec_info_hash, stmt_vec_info_eq,
 				    NULL);
 }
 
@@ -1598,8 +1611,8 @@ free_stmt_vec_info (gimple stmt)
     return;
 
   VEC_free (dr_p, heap, STMT_VINFO_SAME_ALIGN_REFS (stmt_info));
-  free (stmt_info);
   set_vinfo_for_stmt (stmt, NULL);
+  free (stmt_info);
 }
 
 
--- gcc/tree-vectorizer.h.jj	2008-06-27 18:54:54.000000000 +0200
+++ gcc/tree-vectorizer.h	2008-07-01 12:06:42.000000000 +0200
@@ -531,7 +531,8 @@ static inline stmt_vec_info
 vinfo_for_stmt (gimple stmt)
 {
   void **slot;
-  slot = htab_find_slot (stmt_vec_info_htab, stmt, NO_INSERT);
+  slot = htab_find_slot_with_hash (stmt_vec_info_htab, stmt,
+				   htab_hash_pointer (stmt), NO_INSERT);
   return slot ? (stmt_vec_info) *slot : NULL;
 }
 
@@ -539,9 +540,20 @@ static inline void
 set_vinfo_for_stmt (gimple stmt, stmt_vec_info info)
 {
   void **slot;
-  slot = htab_find_slot (stmt_vec_info_htab, stmt, INSERT);
-  gcc_assert (!(*slot && info));
-  *slot = info;
+  if (info)
+    {
+      slot = htab_find_slot_with_hash (stmt_vec_info_htab, stmt,
+				       htab_hash_pointer (stmt), INSERT);
+      gcc_assert (!*slot);
+      *slot = info;
+    }
+  else
+    {
+      slot = htab_find_slot_with_hash (stmt_vec_info_htab, stmt,
+				       htab_hash_pointer (stmt), NO_INSERT);
+      if (slot)
+	htab_clear_slot (stmt_vec_info_htab, slot);
+    }
 }
 
 static inline bool

	Jakub

Re: [tuples] Fix {set_,}vinfo_for_stmt (take 2)

[Diego Novillo]

On Tue, Jul 1, 2008 at 09:08, Jakub Jelinek <jakub@redhat.com> wrote:

> 2008-07-01  Jakub Jelinek  <jakub@redhat.com>
>
>        * tree-vectorizer.h (vinfo_for_stmt): Use htab_find_slot_with_hash.
>        (set_vinfo_for_stmt): Likewise.  If info is NULL, delete entry from
>        hash table.
>        * tree-vectorizer.c (stmt_vec_info_eq, stmt_vec_info_hash): New
>        functions.
>        (init_stmt_vec_info_htab): Use them instead of htab_hash_pointer
>        and htab_eq_pointer.
>        (free_stmt_vec_info): Free stmt_info only after set_vinfo_for_stmt
>        call.

OK.  Thanks for catching and fixing this.


Diego.