Patchwork move htab_iterator

login
register
mail settings
Submitter Andrew MacLeod
Date Sept. 30, 2013, 3:48 p.m.
Message ID <52499D4C.1040507@redhat.com>
Download mbox | patch
Permalink /patch/279178/
State New
Headers show

Comments

Andrew MacLeod - Sept. 30, 2013, 3:48 p.m.
Jakub didn't like removing this hash table iterator functionality even 
though it is currently unused in the compiler.

So it seems reasonable to put it in tree-hasher.h since its purpose is 
"Hash Table Helper for Trees"?

bootstraps on build/x86_64-unknown-linux-gnu with no new regressions.   OK?

Andrew
Tom Tromey - Sept. 30, 2013, 4:08 p.m.
>>>>> "Andrew" == Andrew MacLeod <amacleod@redhat.com> writes:

Andrew> Jakub didn't like removing this hash table iterator functionality even
Andrew> though it is currently unused in the compiler.

Andrew> So it seems reasonable to put it in tree-hasher.h since its purpose is
Andrew> "Hash Table Helper for Trees"?

How about putting it into libiberty?
That way other hashtab users, like gdb, can use it.

Tom
Andrew MacLeod - Sept. 30, 2013, 4:20 p.m.
On 09/30/2013 12:08 PM, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew MacLeod <amacleod@redhat.com> writes:
> Andrew> Jakub didn't like removing this hash table iterator functionality even
> Andrew> though it is currently unused in the compiler.
>
> Andrew> So it seems reasonable to put it in tree-hasher.h since its purpose is
> Andrew> "Hash Table Helper for Trees"?
>
> How about putting it into libiberty?
> That way other hashtab users, like gdb, can use it.
>
>
I have no problem with that, but Jakub didn't seem to think it belonged 
there.

I just want to get it out of where it is :-)  I'll be happy to move it 
to where ever it will have a better home than the non-existence I 
originally planned for it :-)

Andrew
Tom Tromey - Sept. 30, 2013, 5:02 p.m.
Tom> How about putting it into libiberty?
Tom> That way other hashtab users, like gdb, can use it.

Andrew> I have no problem with that, but Jakub didn't seem to think it
Andrew> belonged there.

All I found was this:

http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00721.html

Quoting from it: "It doesn't belong to hashtab.h, because that is a
libiberty API, this style of iterators is GCC specific."

I think that's an accurate assessment of the current code, but I don't
see why it has to continue to be that way.

My argument in favor of moving it to libiberty is that other programs
can then use it; and furthermore that since it is tightly tied to the
hashtab implementation, it ought to be maintained there in order to
preserve the module boundary.

So, please reconsider.

thanks,
Tom

Patch


	* tree-flow.h (htab_iterator, FOR_EACH_HTAB_ELEMENT): Move from here.
	* tree-flow-inline.h (first_htab_element, end_htab_p,
	next_htab_element): Also move from here.
	* tree-hasher.h (htab_iterator, FOR_EACH_HTAB_ELEMENT,
	first_htab_element, end_htab_p, next_htab_element): Move to here.

*** a1/tree-flow.h	2013-09-30 10:52:14.823172915 -0400
--- tree-flow.h	2013-09-30 11:18:59.790306890 -0400
*************** struct GTY(()) gimple_df {
*** 92,112 ****
    htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
  };
  
- 
- typedef struct
- {
-   htab_t htab;
-   PTR *slot;
-   PTR *limit;
- } htab_iterator;
- 
- /* Iterate through the elements of hashtable HTAB, using htab_iterator ITER,
-    storing each element in RESULT, which is of type TYPE.  */
- #define FOR_EACH_HTAB_ELEMENT(HTAB, RESULT, TYPE, ITER) \
-   for (RESULT = (TYPE) first_htab_element (&(ITER), (HTAB)); \
- 	!end_htab_p (&(ITER)); \
- 	RESULT = (TYPE) next_htab_element (&(ITER)))
- 
  static inline int get_lineno (const_gimple);
  
  /*---------------------------------------------------------------------------
--- 92,97 ----
*** a1/tree-flow-inline.h	2013-09-30 10:52:14.824172840 -0400
--- tree-flow-inline.h	2013-09-30 11:17:33.996171267 -0400
*************** gimple_vop (const struct function *fun)
*** 42,93 ****
    return fun->gimple_df->vop;
  }
  
- /* Initialize the hashtable iterator HTI to point to hashtable TABLE */
- 
- static inline void *
- first_htab_element (htab_iterator *hti, htab_t table)
- {
-   hti->htab = table;
-   hti->slot = table->entries;
-   hti->limit = hti->slot + htab_size (table);
-   do
-     {
-       PTR x = *(hti->slot);
-       if (x != HTAB_EMPTY_ENTRY && x != HTAB_DELETED_ENTRY)
- 	break;
-     } while (++(hti->slot) < hti->limit);
- 
-   if (hti->slot < hti->limit)
-     return *(hti->slot);
-   return NULL;
- }
- 
- /* Return current non-empty/deleted slot of the hashtable pointed to by HTI,
-    or NULL if we have  reached the end.  */
- 
- static inline bool
- end_htab_p (const htab_iterator *hti)
- {
-   if (hti->slot >= hti->limit)
-     return true;
-   return false;
- }
- 
- /* Advance the hashtable iterator pointed to by HTI to the next element of the
-    hashtable.  */
- 
- static inline void *
- next_htab_element (htab_iterator *hti)
- {
-   while (++(hti->slot) < hti->limit)
-     {
-       PTR x = *(hti->slot);
-       if (x != HTAB_EMPTY_ENTRY && x != HTAB_DELETED_ENTRY)
- 	return x;
-     };
-   return NULL;
- }
- 
  /* Get the number of the next statement uid to be allocated.  */
  static inline unsigned int
  gimple_stmt_max_uid (struct function *fn)
--- 42,47 ----
*** a1/tree-hasher.h	2013-09-30 10:52:14.824172840 -0400
--- tree-hasher.h	2013-09-30 11:17:33.997171769 -0400
*************** int_tree_hasher::equal (const value_type
*** 52,55 ****
--- 52,119 ----
  
  typedef hash_table <int_tree_hasher> int_tree_htab_type;
  
+ 
+ typedef struct
+ {
+   htab_t htab;
+   PTR *slot;
+   PTR *limit;
+ } htab_iterator;
+ 
+ /* Iterate through the elements of hashtable HTAB, using htab_iterator ITER,
+    storing each element in RESULT, which is of type TYPE.  */
+ #define FOR_EACH_HTAB_ELEMENT(HTAB, RESULT, TYPE, ITER) \
+   for (RESULT = (TYPE) first_htab_element (&(ITER), (HTAB)); \
+ 	!end_htab_p (&(ITER)); \
+ 	RESULT = (TYPE) next_htab_element (&(ITER)))
+ 
+ 
+ /* Initialize the hashtable iterator HTI to point to hashtable TABLE */
+ 
+ static inline void *
+ first_htab_element (htab_iterator *hti, htab_t table)
+ {
+   hti->htab = table;
+   hti->slot = table->entries;
+   hti->limit = hti->slot + htab_size (table);
+   do
+     {
+       PTR x = *(hti->slot);
+       if (x != HTAB_EMPTY_ENTRY && x != HTAB_DELETED_ENTRY)
+ 	break;
+     } while (++(hti->slot) < hti->limit);
+ 
+   if (hti->slot < hti->limit)
+     return *(hti->slot);
+   return NULL;
+ }
+ 
+ /* Return current non-empty/deleted slot of the hashtable pointed to by HTI,
+    or NULL if we have  reached the end.  */
+ 
+ static inline bool
+ end_htab_p (const htab_iterator *hti)
+ {
+   if (hti->slot >= hti->limit)
+     return true;
+   return false;
+ }
+ 
+ /* Advance the hashtable iterator pointed to by HTI to the next element of the
+    hashtable.  */
+ 
+ static inline void *
+ next_htab_element (htab_iterator *hti)
+ {
+   while (++(hti->slot) < hti->limit)
+     {
+       PTR x = *(hti->slot);
+       if (x != HTAB_EMPTY_ENTRY && x != HTAB_DELETED_ENTRY)
+ 	return x;
+     };
+   return NULL;
+ }
+ 
+ 
+ 
  #endif /* GCC_TREE_HASHER_H  */