[PATCH 0/3] Add htab_eq_string to libiberty

[PATCH 0/3] Add htab_eq_string to libiberty

[Tom Tromey]

The libiberty hash table defines a hash function for strings, but not
an equality function.  This means that various files have had to
implement their own comparison function over the years.

This series resolves this for gcc.  Once this is in, I plan to import
the change into binutils-gdb and apply a similar fix there.

While examining all the uses of htab_hash_string, I found an oddity
related to this in libcpp.  I've filed PR preprocessor/100435 for
this.

Tom

[PATCH 1/3] libiberty: add htab_eq_string

[Tom Tromey]

The libiberty hash table includes a helper function for strings, but
no equality function.  Consequently, this equality function has been
reimplemented a number of times in both the gcc and binutils-gdb
source trees.  This patch adds the function to the libiberty hash
table, as a step toward the goal of removing all the copies.

One change to gcc is included here.  Normally I would have put this in
the next patch, but gensupport.c used the most natural name for its
reimplementation of this function, and this can't coexist with the
extern function in libiberty.

include

	* hashtab.h (htab_eq_string): Declare.

libiberty

	* hashtab.c (htab_eq_string): New function.

gcc

	* gensupport.c (htab_eq_string): Remove.
---
 gcc/gensupport.c    | 8 --------
 include/hashtab.h   | 3 +++
 libiberty/hashtab.c | 7 +++++++
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 0f19bd706646..e1ca06dbc1ec 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -2322,14 +2322,6 @@ gen_reader::handle_unknown_directive (file_location loc, const char *rtx_name)
     process_rtx (x, loc);
 }
 
-/* Comparison function for the mnemonic hash table.  */
-
-static int
-htab_eq_string (const void *s1, const void *s2)
-{
-  return strcmp ((const char*)s1, (const char*)s2) == 0;
-}
-
 /* Add mnemonic STR with length LEN to the mnemonic hash table
    MNEMONIC_HTAB.  A trailing zero end character is appended to STR
    and a permanent heap copy of STR is created.  */
diff --git a/include/hashtab.h b/include/hashtab.h
index b3a6265eeb6e..77c5eec79055 100644
--- a/include/hashtab.h
+++ b/include/hashtab.h
@@ -192,6 +192,9 @@ extern htab_eq htab_eq_pointer;
 /* A hash function for null-terminated strings.  */
 extern hashval_t htab_hash_string (const void *);
 
+/* An equality function for null-terminated strings.  */
+extern int htab_eq_string (const void *, const void *);
+
 /* An iterative hash function for arbitrary data.  */
 extern hashval_t iterative_hash (const void *, size_t, hashval_t);
 /* Shorthand for hashing something with an intrinsic size.  */
diff --git a/libiberty/hashtab.c b/libiberty/hashtab.c
index 0c7208effe11..7c424e8f6cc1 100644
--- a/libiberty/hashtab.c
+++ b/libiberty/hashtab.c
@@ -841,6 +841,13 @@ htab_hash_string (const PTR p)
   return r;
 }
 
+/* An equality function for null-terminated strings.  */
+int
+htab_eq_string (const void *a, const void *b)
+{
+  return strcmp ((const char *) a, (const char *) b) == 0;
+}
+
 /* DERIVED FROM:
 --------------------------------------------------------------------
 lookup2.c, by Bob Jenkins, December 1996, Public Domain.
-- 
2.26.3

[PATCH 2/3] gcc: use htab_eq_string

[Tom Tromey]

This changes one spot in GCC to use the new htab_eq_string function.

gcc

	* gengtype-state.c (read_state): Use htab_eq_string.
	(string_eq): Remove.
---
 gcc/gengtype-state.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c
index 891f2e18a610..a8fde959f4eb 100644
--- a/gcc/gengtype-state.c
+++ b/gcc/gengtype-state.c
@@ -2556,15 +2556,6 @@ equals_type_number (const void *ty1, const void *ty2)
   return type1->state_number == type2->state_number;
 }
 
-static int
-string_eq (const void *a, const void *b)
-{
-  const char *a0 = (const char *)a;
-  const char *b0 = (const char *)b;
-
-  return (strcmp (a0, b0) == 0);
-}
-
 
 /* The function reading the state, called by main from gengtype.c.  */
 void
@@ -2588,7 +2579,7 @@ read_state (const char *path)
   state_seen_types =
     htab_create (2017, hash_type_number, equals_type_number, NULL);
   state_ident_tab =
-    htab_create (4027, htab_hash_string, string_eq, NULL);
+    htab_create (4027, htab_hash_string, htab_eq_string, NULL);
   read_state_version (version_string);
   read_state_srcdir ();
   read_state_languages ();
-- 
2.26.3

[PATCH 3/3] go: use htab_eq_string in godump

[Tom Tromey]

This changes godump to use the new htab_eq_string function.

gcc

	* godump.c (string_hash_eq): Remove.
	(go_finish): Use htab_eq_string.
---
 gcc/godump.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/gcc/godump.c b/gcc/godump.c
index 7864d9d63e5d..cf9989490356 100644
--- a/gcc/godump.c
+++ b/gcc/godump.c
@@ -109,14 +109,6 @@ macro_hash_del (void *v)
   XDELETE (mhv);
 }
 
-/* For the string hash tables.  */
-
-static int
-string_hash_eq (const void *y1, const void *y2)
-{
-  return strcmp ((const char *) y1, (const char *) y2) == 0;
-}
-
 /* A macro definition.  */
 
 static void
@@ -1374,11 +1366,11 @@ go_finish (const char *filename)
   real_debug_hooks->finish (filename);
 
   container.type_hash = htab_create (100, htab_hash_string,
-                                     string_hash_eq, NULL);
+				     htab_eq_string, NULL);
   container.invalid_hash = htab_create (10, htab_hash_string,
-					string_hash_eq, NULL);
+					htab_eq_string, NULL);
   container.keyword_hash = htab_create (50, htab_hash_string,
-                                        string_hash_eq, NULL);
+					htab_eq_string, NULL);
   obstack_init (&container.type_obstack);
 
   keyword_hash_init (&container);
-- 
2.26.3

Re: [PATCH 1/3] libiberty: add htab_eq_string

[Richard Biener]

On Thu, May 6, 2021 at 12:41 AM Tom Tromey <tom@tromey.com> wrote:
>
> The libiberty hash table includes a helper function for strings, but
> no equality function.  Consequently, this equality function has been
> reimplemented a number of times in both the gcc and binutils-gdb
> source trees.  This patch adds the function to the libiberty hash
> table, as a step toward the goal of removing all the copies.
>
> One change to gcc is included here.  Normally I would have put this in
> the next patch, but gensupport.c used the most natural name for its
> reimplementation of this function, and this can't coexist with the
> extern function in libiberty.

OK.

> include
>
>         * hashtab.h (htab_eq_string): Declare.
>
> libiberty
>
>         * hashtab.c (htab_eq_string): New function.
>
> gcc
>
>         * gensupport.c (htab_eq_string): Remove.
> ---
>  gcc/gensupport.c    | 8 --------
>  include/hashtab.h   | 3 +++
>  libiberty/hashtab.c | 7 +++++++
>  3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/gensupport.c b/gcc/gensupport.c
> index 0f19bd706646..e1ca06dbc1ec 100644
> --- a/gcc/gensupport.c
> +++ b/gcc/gensupport.c
> @@ -2322,14 +2322,6 @@ gen_reader::handle_unknown_directive (file_location loc, const char *rtx_name)
>      process_rtx (x, loc);
>  }
>
> -/* Comparison function for the mnemonic hash table.  */
> -
> -static int
> -htab_eq_string (const void *s1, const void *s2)
> -{
> -  return strcmp ((const char*)s1, (const char*)s2) == 0;
> -}
> -
>  /* Add mnemonic STR with length LEN to the mnemonic hash table
>     MNEMONIC_HTAB.  A trailing zero end character is appended to STR
>     and a permanent heap copy of STR is created.  */
> diff --git a/include/hashtab.h b/include/hashtab.h
> index b3a6265eeb6e..77c5eec79055 100644
> --- a/include/hashtab.h
> +++ b/include/hashtab.h
> @@ -192,6 +192,9 @@ extern htab_eq htab_eq_pointer;
>  /* A hash function for null-terminated strings.  */
>  extern hashval_t htab_hash_string (const void *);
>
> +/* An equality function for null-terminated strings.  */
> +extern int htab_eq_string (const void *, const void *);
> +
>  /* An iterative hash function for arbitrary data.  */
>  extern hashval_t iterative_hash (const void *, size_t, hashval_t);
>  /* Shorthand for hashing something with an intrinsic size.  */
> diff --git a/libiberty/hashtab.c b/libiberty/hashtab.c
> index 0c7208effe11..7c424e8f6cc1 100644
> --- a/libiberty/hashtab.c
> +++ b/libiberty/hashtab.c
> @@ -841,6 +841,13 @@ htab_hash_string (const PTR p)
>    return r;
>  }
>
> +/* An equality function for null-terminated strings.  */
> +int
> +htab_eq_string (const void *a, const void *b)
> +{
> +  return strcmp ((const char *) a, (const char *) b) == 0;
> +}
> +
>  /* DERIVED FROM:
>  --------------------------------------------------------------------
>  lookup2.c, by Bob Jenkins, December 1996, Public Domain.
> --
> 2.26.3
>

Re: [PATCH 2/3] gcc: use htab_eq_string

[Richard Biener]

On Thu, May 6, 2021 at 12:43 AM Tom Tromey <tom@tromey.com> wrote:
>
> This changes one spot in GCC to use the new htab_eq_string function.

OK.

> gcc
>
>         * gengtype-state.c (read_state): Use htab_eq_string.
>         (string_eq): Remove.
> ---
>  gcc/gengtype-state.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c
> index 891f2e18a610..a8fde959f4eb 100644
> --- a/gcc/gengtype-state.c
> +++ b/gcc/gengtype-state.c
> @@ -2556,15 +2556,6 @@ equals_type_number (const void *ty1, const void *ty2)
>    return type1->state_number == type2->state_number;
>  }
>
> -static int
> -string_eq (const void *a, const void *b)
> -{
> -  const char *a0 = (const char *)a;
> -  const char *b0 = (const char *)b;
> -
> -  return (strcmp (a0, b0) == 0);
> -}
> -
>
>  /* The function reading the state, called by main from gengtype.c.  */
>  void
> @@ -2588,7 +2579,7 @@ read_state (const char *path)
>    state_seen_types =
>      htab_create (2017, hash_type_number, equals_type_number, NULL);
>    state_ident_tab =
> -    htab_create (4027, htab_hash_string, string_eq, NULL);
> +    htab_create (4027, htab_hash_string, htab_eq_string, NULL);
>    read_state_version (version_string);
>    read_state_srcdir ();
>    read_state_languages ();
> --
> 2.26.3
>

Re: [PATCH 3/3] go: use htab_eq_string in godump

[Richard Biener]

On Thu, May 6, 2021 at 12:44 AM Tom Tromey <tom@tromey.com> wrote:
>
> This changes godump to use the new htab_eq_string function.

OK.

> gcc
>
>         * godump.c (string_hash_eq): Remove.
>         (go_finish): Use htab_eq_string.
> ---
>  gcc/godump.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/gcc/godump.c b/gcc/godump.c
> index 7864d9d63e5d..cf9989490356 100644
> --- a/gcc/godump.c
> +++ b/gcc/godump.c
> @@ -109,14 +109,6 @@ macro_hash_del (void *v)
>    XDELETE (mhv);
>  }
>
> -/* For the string hash tables.  */
> -
> -static int
> -string_hash_eq (const void *y1, const void *y2)
> -{
> -  return strcmp ((const char *) y1, (const char *) y2) == 0;
> -}
> -
>  /* A macro definition.  */
>
>  static void
> @@ -1374,11 +1366,11 @@ go_finish (const char *filename)
>    real_debug_hooks->finish (filename);
>
>    container.type_hash = htab_create (100, htab_hash_string,
> -                                     string_hash_eq, NULL);
> +                                    htab_eq_string, NULL);
>    container.invalid_hash = htab_create (10, htab_hash_string,
> -                                       string_hash_eq, NULL);
> +                                       htab_eq_string, NULL);
>    container.keyword_hash = htab_create (50, htab_hash_string,
> -                                        string_hash_eq, NULL);
> +                                       htab_eq_string, NULL);
>    obstack_init (&container.type_obstack);
>
>    keyword_hash_init (&container);
> --
> 2.26.3
>