public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
@ 2018-10-31 17:32 Martin Liška
  2018-11-02  8:03 ` Richard Biener
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2018-10-31 17:32 UTC (permalink / raw)
  To: gcc-patches

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

Hi.

As seen in r265663 having htab_hash_string accepting const char * would
report a compilation error. The void * argument is needed for old C-style
htab used in libiberty. I'm suggesting to come up with htab_hash_string_vptr and
change signature of the old one (htab_hash_string). And putting these into
hashtab.h will make it inlinable in C++-style hash table.

Patch survives regression tests on ppc64le-linux-gnu.
Total cc1 change with the patch:
  +0.0% +2.09Ki

Hope it's acceptable.
Thanks,
Martin

gcc/ChangeLog:

2018-10-31  Martin Liska  <mliska@suse.cz>

	* gengtype-state.c (read_state): Use newly added
	htab_hash_string_vptr.
	* gensupport.c (gen_mnemonic_attr): Likewise.
	(check_define_attr_duplicates): Likewise.
	* godump.c (go_finish): Likewise.

include/ChangeLog:

2018-10-31  Martin Liska  <mliska@suse.cz>

	* hashtab.h (htab_hash_string): Change signature
	to const char * and make it static inline.
	(htab_hash_string_vptr): Likewise.

libcpp/ChangeLog:

2018-10-31  Martin Liska  <mliska@suse.cz>

	* files.c (_cpp_init_files): Use htab_hash_string_vptr.

libiberty/ChangeLog:

2018-10-31  Martin Liska  <mliska@suse.cz>

	* hashtab.c:
	(htab_hash_string): Move to header file.
---
 gcc/gengtype-state.c |  2 +-
 gcc/gensupport.c     |  4 ++--
 gcc/godump.c         |  6 +++---
 include/hashtab.h    | 47 ++++++++++++++++++++++++++++++++++++++++++--
 libcpp/files.c       |  2 +-
 libiberty/hashtab.c  | 38 -----------------------------------
 6 files changed, 52 insertions(+), 47 deletions(-)



[-- Attachment #2: 0001-Come-up-with-htab_hash_string_vptr-and-use-string-sp.patch --]
[-- Type: text/x-patch, Size: 6601 bytes --]

diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c
index 6a3b981eedc..facff4a29d7 100644
--- a/gcc/gengtype-state.c
+++ b/gcc/gengtype-state.c
@@ -2588,7 +2588,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_vptr, string_eq, NULL);
   read_state_version (version_string);
   read_state_srcdir ();
   read_state_languages ();
diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 1d76af7ce5c..2f865d918f3 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -2434,7 +2434,7 @@ gen_mnemonic_attr (void)
   if (!mnemonic_attr)
     return;
 
-  mnemonic_htab = htab_create_alloc (MNEMONIC_HTAB_SIZE, htab_hash_string,
+  mnemonic_htab = htab_create_alloc (MNEMONIC_HTAB_SIZE, htab_hash_string_vptr,
 				     htab_eq_string, 0, xcalloc, free);
 
   for (elem = define_insn_queue; elem; elem = elem->next)
@@ -2492,7 +2492,7 @@ check_define_attr_duplicates ()
   char * attr_name;
   void **slot;
 
-  attr_htab = htab_create (500, htab_hash_string, htab_eq_string, NULL);
+  attr_htab = htab_create (500, htab_hash_string_vptr, htab_eq_string, NULL);
 
   for (elem = define_attr_queue; elem; elem = elem->next)
     {
diff --git a/gcc/godump.c b/gcc/godump.c
index baf21e27eec..7098ac80a91 100644
--- a/gcc/godump.c
+++ b/gcc/godump.c
@@ -1377,11 +1377,11 @@ go_finish (const char *filename)
 
   real_debug_hooks->finish (filename);
 
-  container.type_hash = htab_create (100, htab_hash_string,
+  container.type_hash = htab_create (100, htab_hash_string_vptr,
                                      string_hash_eq, NULL);
-  container.invalid_hash = htab_create (10, htab_hash_string,
+  container.invalid_hash = htab_create (10, htab_hash_string_vptr,
 					string_hash_eq, NULL);
-  container.keyword_hash = htab_create (50, htab_hash_string,
+  container.keyword_hash = htab_create (50, htab_hash_string_vptr,
                                         string_hash_eq, NULL);
   obstack_init (&container.type_obstack);
 
diff --git a/include/hashtab.h b/include/hashtab.h
index 531eb101ff7..44bce8c2a6b 100644
--- a/include/hashtab.h
+++ b/include/hashtab.h
@@ -189,8 +189,51 @@ extern htab_hash htab_hash_pointer;
 /* An equality function for pointers.  */
 extern htab_eq htab_eq_pointer;
 
-/* A hash function for null-terminated strings.  */
-extern hashval_t htab_hash_string (const void *);
+/* Hash STR as a null-terminated string.
+
+   Copied from gcc/hashtable.c.  Zack had the following to say with respect
+   to applicability, though note that unlike hashtable.c, this hash table
+   implementation re-hashes rather than chain buckets.
+
+   http://gcc.gnu.org/ml/gcc-patches/2001-08/msg01021.html
+   From: Zack Weinberg <zackw@panix.com>
+   Date: Fri, 17 Aug 2001 02:15:56 -0400
+
+   I got it by extracting all the identifiers from all the source code
+   I had lying around in mid-1999, and testing many recurrences of
+   the form "H_n = H_{n-1} * K + c_n * L + M" where K, L, M were either
+   prime numbers or the appropriate identity.  This was the best one.
+   I don't remember exactly what constituted "best", except I was
+   looking at bucket-length distributions mostly.
+
+   So it should be very good at hashing identifiers, but might not be
+   as good at arbitrary strings.
+
+   I'll add that it thoroughly trounces the hash functions recommended
+   for this use at http://burtleburtle.net/bob/hash/index.html, both
+   on speed and bucket distribution.  I haven't tried it against the
+   function they just started using for Perl's hashes.  */
+
+static inline hashval_t
+htab_hash_string (const char *str)
+{
+  hashval_t r = 0;
+  unsigned char c;
+
+  while ((c = *str++) != 0)
+    r = r * 67 + c - 113;
+
+  return r;
+}
+
+/* Hash P as a null-terminated string.  */
+
+static inline hashval_t
+htab_hash_string_vptr (const PTR p)
+{
+  return htab_hash_string ((const char *)p);
+}
+
 
 /* An iterative hash function for arbitrary data.  */
 extern hashval_t iterative_hash (const void *, size_t, hashval_t);
diff --git a/libcpp/files.c b/libcpp/files.c
index 08b7c647c91..e4eb34e7d55 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -1311,7 +1311,7 @@ _cpp_init_files (cpp_reader *pfile)
   pfile->dir_hash = htab_create_alloc (127, file_hash_hash, file_hash_eq,
 					NULL, xcalloc, free);
   allocate_file_hash_entries (pfile);
-  pfile->nonexistent_file_hash = htab_create_alloc (127, htab_hash_string,
+  pfile->nonexistent_file_hash = htab_create_alloc (127, htab_hash_string_vptr,
 						    nonexistent_file_hash_eq,
 						    NULL, xcalloc, free);
   obstack_specify_allocation (&pfile->nonexistent_file_ob, 0, 0,
diff --git a/libiberty/hashtab.c b/libiberty/hashtab.c
index ca392647d4a..b6821502d15 100644
--- a/libiberty/hashtab.c
+++ b/libiberty/hashtab.c
@@ -803,44 +803,6 @@ htab_collisions (htab_t htab)
   return (double) htab->collisions / (double) htab->searches;
 }
 
-/* Hash P as a null-terminated string.
-
-   Copied from gcc/hashtable.c.  Zack had the following to say with respect
-   to applicability, though note that unlike hashtable.c, this hash table
-   implementation re-hashes rather than chain buckets.
-
-   http://gcc.gnu.org/ml/gcc-patches/2001-08/msg01021.html
-   From: Zack Weinberg <zackw@panix.com>
-   Date: Fri, 17 Aug 2001 02:15:56 -0400
-
-   I got it by extracting all the identifiers from all the source code
-   I had lying around in mid-1999, and testing many recurrences of
-   the form "H_n = H_{n-1} * K + c_n * L + M" where K, L, M were either
-   prime numbers or the appropriate identity.  This was the best one.
-   I don't remember exactly what constituted "best", except I was
-   looking at bucket-length distributions mostly.
-   
-   So it should be very good at hashing identifiers, but might not be
-   as good at arbitrary strings.
-   
-   I'll add that it thoroughly trounces the hash functions recommended
-   for this use at http://burtleburtle.net/bob/hash/index.html, both
-   on speed and bucket distribution.  I haven't tried it against the
-   function they just started using for Perl's hashes.  */
-
-hashval_t
-htab_hash_string (const PTR p)
-{
-  const unsigned char *str = (const unsigned char *) p;
-  hashval_t r = 0;
-  unsigned char c;
-
-  while ((c = *str++) != 0)
-    r = r * 67 + c - 113;
-
-  return r;
-}
-
 /* DERIVED FROM:
 --------------------------------------------------------------------
 lookup2.c, by Bob Jenkins, December 1996, Public Domain.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-10-31 17:32 [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible Martin Liška
@ 2018-11-02  8:03 ` Richard Biener
  2018-11-02  8:10   ` Martin Liška
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2018-11-02  8:03 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches

On Wed, Oct 31, 2018 at 5:40 PM Martin Liška <mliska@suse.cz> wrote:
>
> Hi.
>
> As seen in r265663 having htab_hash_string accepting const char * would
> report a compilation error. The void * argument is needed for old C-style
> htab used in libiberty. I'm suggesting to come up with htab_hash_string_vptr and
> change signature of the old one (htab_hash_string). And putting these into
> hashtab.h will make it inlinable in C++-style hash table.
>
> Patch survives regression tests on ppc64le-linux-gnu.
> Total cc1 change with the patch:
>   +0.0% +2.09Ki
>
> Hope it's acceptable.

What's the reason to inline the implementation?  I guess you want to
get a compilation error when calling htab_hash_string on a non-string?
But then libiberty isn't C++ an is used in other projects which you
may be breaking with your patch?

A solution might be to poison htab_hash_string and provide our own
variant.

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2018-10-31  Martin Liska  <mliska@suse.cz>
>
>         * gengtype-state.c (read_state): Use newly added
>         htab_hash_string_vptr.
>         * gensupport.c (gen_mnemonic_attr): Likewise.
>         (check_define_attr_duplicates): Likewise.
>         * godump.c (go_finish): Likewise.
>
> include/ChangeLog:
>
> 2018-10-31  Martin Liska  <mliska@suse.cz>
>
>         * hashtab.h (htab_hash_string): Change signature
>         to const char * and make it static inline.
>         (htab_hash_string_vptr): Likewise.
>
> libcpp/ChangeLog:
>
> 2018-10-31  Martin Liska  <mliska@suse.cz>
>
>         * files.c (_cpp_init_files): Use htab_hash_string_vptr.
>
> libiberty/ChangeLog:
>
> 2018-10-31  Martin Liska  <mliska@suse.cz>
>
>         * hashtab.c:
>         (htab_hash_string): Move to header file.
> ---
>  gcc/gengtype-state.c |  2 +-
>  gcc/gensupport.c     |  4 ++--
>  gcc/godump.c         |  6 +++---
>  include/hashtab.h    | 47 ++++++++++++++++++++++++++++++++++++++++++--
>  libcpp/files.c       |  2 +-
>  libiberty/hashtab.c  | 38 -----------------------------------
>  6 files changed, 52 insertions(+), 47 deletions(-)
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-02  8:03 ` Richard Biener
@ 2018-11-02  8:10   ` Martin Liška
  2018-11-02  9:25     ` Martin Liška
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2018-11-02  8:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On 11/2/18 9:02 AM, Richard Biener wrote:
> On Wed, Oct 31, 2018 at 5:40 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> Hi.
>>
>> As seen in r265663 having htab_hash_string accepting const char * would
>> report a compilation error. The void * argument is needed for old C-style
>> htab used in libiberty. I'm suggesting to come up with htab_hash_string_vptr and
>> change signature of the old one (htab_hash_string). And putting these into
>> hashtab.h will make it inlinable in C++-style hash table.
>>
>> Patch survives regression tests on ppc64le-linux-gnu.
>> Total cc1 change with the patch:
>>   +0.0% +2.09Ki
>>
>> Hope it's acceptable.
> 
> What's the reason to inline the implementation?

Doing that can enable inlining of hash_table::* functions (find_slot_with_hash, ...)
in gcc.

I guess you want to
> get a compilation error when calling htab_hash_string on a non-string?

Yep.

> But then libiberty isn't C++ an is used in other projects which you
> may be breaking with your patch?

Ah, ok, that's new for me that it's used elsewhere :)

> 
> A solution might be to poison htab_hash_string and provide our own
> variant.

Will work on that.

Martin

> 
> Richard.
> 
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-10-31  Martin Liska  <mliska@suse.cz>
>>
>>         * gengtype-state.c (read_state): Use newly added
>>         htab_hash_string_vptr.
>>         * gensupport.c (gen_mnemonic_attr): Likewise.
>>         (check_define_attr_duplicates): Likewise.
>>         * godump.c (go_finish): Likewise.
>>
>> include/ChangeLog:
>>
>> 2018-10-31  Martin Liska  <mliska@suse.cz>
>>
>>         * hashtab.h (htab_hash_string): Change signature
>>         to const char * and make it static inline.
>>         (htab_hash_string_vptr): Likewise.
>>
>> libcpp/ChangeLog:
>>
>> 2018-10-31  Martin Liska  <mliska@suse.cz>
>>
>>         * files.c (_cpp_init_files): Use htab_hash_string_vptr.
>>
>> libiberty/ChangeLog:
>>
>> 2018-10-31  Martin Liska  <mliska@suse.cz>
>>
>>         * hashtab.c:
>>         (htab_hash_string): Move to header file.
>> ---
>>  gcc/gengtype-state.c |  2 +-
>>  gcc/gensupport.c     |  4 ++--
>>  gcc/godump.c         |  6 +++---
>>  include/hashtab.h    | 47 ++++++++++++++++++++++++++++++++++++++++++--
>>  libcpp/files.c       |  2 +-
>>  libiberty/hashtab.c  | 38 -----------------------------------
>>  6 files changed, 52 insertions(+), 47 deletions(-)
>>
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-02  8:10   ` Martin Liška
@ 2018-11-02  9:25     ` Martin Liška
  2018-11-05 18:37       ` Michael Matz
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2018-11-02  9:25 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

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

V2 of the patch.

Thoughts?
Martin

[-- Attachment #2: 0001-Come-up-with-htab_hash_string_vptr-and-use-string-sp.patch --]
[-- Type: text/x-patch, Size: 27268 bytes --]

From 484d6d29292f210946f9d5091273eb9e1796c874 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 2 Nov 2018 10:15:10 +0100
Subject: [PATCH] Come up with htab_hash_string_vptr and use string-specific if
 possible.

gcc/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* system.h (hash_string): New function.
	(hash_string_vptr): Likewise.
	Poison htab_hash_string.
	* attribs.c (struct excl_hash_traits): Use hash_string and
	hash_string_vptr instead of libiberty htah_hash_string.
	* config/darwin.c (indirection_hasher::hash): Likewise.
	(machopic_indirection_name): Likewise.
	(machopic_validate_stub_or_non_lazy_ptr): Likewise.
	* config/sol2.c (comdat_entry_hasher::hash): Likewise.
	* dwarf2out.c (indirect_string_hasher::hash): Likewise.
	(find_AT_string_in_table): Likewise.
	(addr_hasher::hash): Likewise.
	(external_ref_hasher::hash): Likewise.
	(dwarf_file_hasher::hash): Likewise.
	(lookup_filename): Likewise.
	(macinfo_entry_hasher::hash): Likewise.
	(prune_unused_types_update_strings): Likewise.
	* gengtype-state.c (read_state): Likewise.
	* gengtype.c (htab_hash_inputfile): Likewise.
	* genhooks.c (s_hook_hash): Likewise.
	* genmatch.c (id_base::id_base): Likewise.
	* genmodes.c (hash_mode): Likewise.
	* gensupport.c (gen_mnemonic_attr): Likewise.
	(check_define_attr_duplicates): Likewise.
	(hash_struct_pred_data): Likewise.
	* gentarget-def.c (insn_hasher::hash): Likewise.
	(def_target_insn): Likewise.
	(add_insn): Likewise.
	* godump.c (macro_hash_hashval): Likewise.
	(go_define): Likewise.
	(go_finish): Likewise.
	* hash-traits.h (string_hash::hash): Likewise.
	* lra.c (lra_rtx_hash): Likewise.
	* lto-section-in.c (hash_name): Likewise.
	* plugin.c (event_hasher::hash): Likewise.
	(htab_hash_plugin): Likewise.
	(add_new_plugin): Likewise.
	(parse_plugin_arg_opt): Likewise.
	(register_plugin_info): Likewise.
	(init_one_plugin): Likewise.
	* read-md.c (leading_string_hash): Likewise.
	* read-rtl.c (overloaded_name_hash): Likewise.
	* statistics.c (stats_counter_hasher::hash): Likewise.
	* symtab.c (symbol_table::decl_assembler_name_hash): Likewise.
	(section_name_hasher::hash): Likewise.
	(symtab_node::set_section_for_node): Likewise.
	* tlink.c (hash_string_hash): Likewise.
	(symbol_hash_lookup): Likewise.
	(file_hash_lookup): Likewise.
	(demangled_hash_lookup): Likewise.
	* varasm.c (section_hasher::hash): Likewise.
	(hash_section): Likewise.
	(get_section): Likewise.
	(const_rtx_hash_1): Likewise.

gcc/cp/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* cp-ubsan.c (cp_ubsan_instrument_vptr): Use hash_string and
	hash_string_vptr instead of libiberty htah_hash_string.

gcc/fortran/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* trans-decl.c (struct module_hasher): Use hash_string and
	hash_string_vptr instead of libiberty htah_hash_string.
	(module_decl_hasher::hash): Likewise.
	(gfc_find_module): Likewise.
	(gfc_module_add_decl): Likewise.
	(gfc_trans_use_stmts): Likewise.

gcc/lto/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* lto.c (hash_name): Use hash_string and
	hash_string_vptr instead of libiberty htah_hash_string.

libcc1/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* libcc1plugin.cc (struct string_hasher): Use hash_string and
	hash_string_vptr instead of libiberty htah_hash_string.
	* libcp1plugin.cc (struct string_hasher): Likewise.
---
 gcc/attribs.c            |  4 ++--
 gcc/config/darwin.c      |  6 +++---
 gcc/config/sol2.c        |  2 +-
 gcc/cp/cp-ubsan.c        |  2 +-
 gcc/dwarf2out.c          | 16 ++++++++--------
 gcc/fortran/trans-decl.c | 10 +++++-----
 gcc/gengtype-state.c     |  2 +-
 gcc/gengtype.c           |  2 +-
 gcc/genhooks.c           |  2 +-
 gcc/genmatch.c           |  2 +-
 gcc/genmodes.c           |  2 +-
 gcc/gensupport.c         |  6 +++---
 gcc/gentarget-def.c      |  6 +++---
 gcc/godump.c             | 10 +++++-----
 gcc/hash-traits.h        |  2 +-
 gcc/lra.c                |  2 +-
 gcc/lto-section-in.c     |  2 +-
 gcc/lto/lto.c            |  2 +-
 gcc/plugin.c             | 12 ++++++------
 gcc/read-md.c            |  2 +-
 gcc/read-rtl.c           |  2 +-
 gcc/statistics.c         |  2 +-
 gcc/symtab.c             | 10 +++++-----
 gcc/system.h             | 24 ++++++++++++++++++++++++
 gcc/tlink.c              |  8 ++++----
 gcc/varasm.c             |  8 ++++----
 libcc1/libcc1plugin.cc   |  2 +-
 libcc1/libcp1plugin.cc   |  2 +-
 28 files changed, 88 insertions(+), 64 deletions(-)

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 8b721274d3b..11fe1e51b30 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -1828,8 +1828,8 @@ struct excl_hash_traits: typed_noop_remove<excl_pair>
 
   static hashval_t hash (const value_type &x)
   {
-    hashval_t h1 = htab_hash_string (x.first);
-    hashval_t h2 = htab_hash_string (x.second);
+    hashval_t h1 = hash_string (x.first);
+    hashval_t h2 = hash_string (x.second);
     return h1 ^ h2;
   }
 
diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
index aa2ef91c64a..1c1b930c174 100644
--- a/gcc/config/darwin.c
+++ b/gcc/config/darwin.c
@@ -473,7 +473,7 @@ static GTY (()) hash_table<indirection_hasher> *machopic_indirections;
 hashval_t
 indirection_hasher::hash (machopic_indirection *p)
 {
-  return htab_hash_string (p->ptr_name);
+  return hash_string (p->ptr_name);
 }
 
 /* Returns true if the KEY is the same as that associated with
@@ -577,7 +577,7 @@ machopic_indirection_name (rtx sym_ref, bool stub_p)
 
   machopic_indirection **slot
     = machopic_indirections->find_slot_with_hash (buffer,
-						  htab_hash_string (buffer),
+						  hash_string (buffer),
 						  INSERT);
   if (*slot)
     {
@@ -614,7 +614,7 @@ void
 machopic_validate_stub_or_non_lazy_ptr (const char *name)
 {
   machopic_indirection *p
-    = machopic_indirections->find_with_hash (name, htab_hash_string (name));
+    = machopic_indirections->find_with_hash (name, hash_string (name));
   if (p && ! p->used)
     {
       const char *real_name;
diff --git a/gcc/config/sol2.c b/gcc/config/sol2.c
index 2043326551d..9e262dc3544 100644
--- a/gcc/config/sol2.c
+++ b/gcc/config/sol2.c
@@ -183,7 +183,7 @@ struct comdat_entry_hasher : nofree_ptr_hash <comdat_entry>
 inline hashval_t
 comdat_entry_hasher::hash (const comdat_entry *entry)
 {
-  return htab_hash_string (entry->sig);
+  return hash_string (entry->sig);
 }
 
 inline bool
diff --git a/gcc/cp/cp-ubsan.c b/gcc/cp/cp-ubsan.c
index 0c02db48e47..75fce10bf09 100644
--- a/gcc/cp/cp-ubsan.c
+++ b/gcc/cp/cp-ubsan.c
@@ -61,7 +61,7 @@ cp_ubsan_instrument_vptr (location_t loc, tree op, tree type, bool is_addr,
 {
   type = TYPE_MAIN_VARIANT (type);
   const char *mangled = mangle_type_string (type);
-  hashval_t str_hash1 = htab_hash_string (mangled);
+  hashval_t str_hash1 = hash_string (mangled);
   hashval_t str_hash2 = iterative_hash (mangled, strlen (mangled), 0);
   tree str_hash = wide_int_to_tree (uint64_type_node,
 				    wi::uhwi (((uint64_t) str_hash1 << 32)
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 30bbfee9052..4cb16e4ba93 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -4603,7 +4603,7 @@ add_AT_low_high_pc (dw_die_ref die, const char *lbl_low, const char *lbl_high,
 hashval_t
 indirect_string_hasher::hash (indirect_string_node *x)
 {
-  return htab_hash_string (x->str);
+  return hash_string (x->str);
 }
 
 bool
@@ -4621,7 +4621,7 @@ find_AT_string_in_table (const char *str,
   struct indirect_string_node *node;
 
   indirect_string_node **slot
-    = table->find_slot_with_hash (str, htab_hash_string (str), INSERT);
+    = table->find_slot_with_hash (str, hash_string (str), INSERT);
   if (*slot == NULL)
     {
       node = ggc_cleared_alloc<indirect_string_node> ();
@@ -4955,7 +4955,7 @@ addr_hasher::hash (addr_table_entry *a)
 	hstate.add_int (1);
 	break;
       case ate_kind_label:
-        return htab_hash_string (a->addr.label);
+	return hash_string (a->addr.label);
       default:
         gcc_unreachable ();
     }
@@ -8856,7 +8856,7 @@ external_ref_hasher::hash (const external_ref *r)
      that will make the order of the stub DIEs non-deterministic.  */
   if (! die->comdat_type_p)
     /* We have a symbol; use it to compute a hash.  */
-    h = htab_hash_string (die->die_id.die_symbol);
+    h = hash_string (die->die_id.die_symbol);
   else
     {
       /* We have a type signature; use a subset of the bits as the hash.
@@ -26861,7 +26861,7 @@ dwarf_file_hasher::equal (dwarf_file_data *p1, const char *p2)
 hashval_t
 dwarf_file_hasher::hash (dwarf_file_data *p)
 {
-  return htab_hash_string (p->filename);
+  return hash_string (p->filename);
 }
 
 /* Lookup FILE_NAME (in the list of filenames that we know about here in
@@ -26882,7 +26882,7 @@ lookup_filename (const char *file_name)
     return NULL;
 
   dwarf_file_data **slot
-    = file_table->find_slot_with_hash (file_name, htab_hash_string (file_name),
+    = file_table->find_slot_with_hash (file_name, hash_string (file_name),
 				       INSERT);
   if (*slot)
     return *slot;
@@ -28021,7 +28021,7 @@ struct macinfo_entry_hasher : nofree_ptr_hash <macinfo_entry>
 inline hashval_t
 macinfo_entry_hasher::hash (const macinfo_entry *entry)
 {
-  return htab_hash_string (entry->info);
+  return hash_string (entry->info);
 }
 
 inline bool
@@ -29259,7 +29259,7 @@ prune_unused_types_update_strings (dw_die_ref die)
 	  {
 	    indirect_string_node **slot
 	      = debug_str_hash->find_slot_with_hash (s->str,
-						     htab_hash_string (s->str),
+						     hash_string (s->str),
 						     INSERT);
 	    gcc_assert (*slot == NULL);
 	    *slot = s;
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 83cbcd123fe..4b7ec16e9a5 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -4827,7 +4827,7 @@ struct module_hasher : ggc_ptr_hash<module_htab_entry>
 
   static hashval_t hash (module_htab_entry *s)
   {
-    return htab_hash_string (s->name);
+    return hash_string (s->name);
   }
 
   static bool
@@ -4847,7 +4847,7 @@ module_decl_hasher::hash (tree t)
   const_tree n = DECL_NAME (t);
   if (n == NULL_TREE)
     n = TYPE_NAME (TREE_TYPE (t));
-  return htab_hash_string (IDENTIFIER_POINTER (n));
+  return hash_string (IDENTIFIER_POINTER (n));
 }
 
 bool
@@ -4866,7 +4866,7 @@ gfc_find_module (const char *name)
     module_htab = hash_table<module_hasher>::create_ggc (10);
 
   module_htab_entry **slot
-    = module_htab->find_slot_with_hash (name, htab_hash_string (name), INSERT);
+    = module_htab->find_slot_with_hash (name, hash_string (name), INSERT);
   if (*slot == NULL)
     {
       module_htab_entry *entry = ggc_cleared_alloc<module_htab_entry> ();
@@ -4891,7 +4891,7 @@ gfc_module_add_decl (struct module_htab_entry *entry, tree decl)
       name = IDENTIFIER_POINTER (TYPE_NAME (TREE_TYPE (decl)));
     }
   tree *slot
-    = entry->decls->find_slot_with_hash (name, htab_hash_string (name),
+    = entry->decls->find_slot_with_hash (name, hash_string (name),
 					 INSERT);
   if (*slot == NULL)
     *slot = decl;
@@ -5079,7 +5079,7 @@ gfc_trans_use_stmts (gfc_namespace * ns)
 	  if (rent->op != INTRINSIC_NONE)
 	    continue;
 
-						 hashval_t hash = htab_hash_string (rent->use_name);
+	  hashval_t hash = hash_string (rent->use_name);
 	  tree *slot = entry->decls->find_slot_with_hash (rent->use_name, hash,
 							  INSERT);
 	  if (*slot == NULL)
diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c
index 6a3b981eedc..d1cbf23dc6a 100644
--- a/gcc/gengtype-state.c
+++ b/gcc/gengtype-state.c
@@ -2588,7 +2588,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, hash_string_vptr, string_eq, NULL);
   read_state_version (version_string);
   read_state_srcdir ();
   read_state_languages ();
diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index 4339456e9c6..a4e7bf59c72 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -5128,7 +5128,7 @@ htab_hash_inputfile (const void *p)
 {
   const input_file *inpf = (const input_file *) p;
   gcc_assert (inpf);
-  return htab_hash_string (get_input_file_name (inpf));
+  return hash_string (get_input_file_name (inpf));
 }
 
 static int
diff --git a/gcc/genhooks.c b/gcc/genhooks.c
index 9b6ab05507c..24d758c7768 100644
--- a/gcc/genhooks.c
+++ b/gcc/genhooks.c
@@ -80,7 +80,7 @@ static hashval_t
 s_hook_hash (const void *p)
 {
   const struct s_hook *s_hook = (const struct s_hook *)p;
-  return htab_hash_string (s_hook->name);
+  return hash_string (s_hook->name);
 }
 
 static int
diff --git a/gcc/genmatch.c b/gcc/genmatch.c
index 62a876dc3da..ee0301e2c81 100644
--- a/gcc/genmatch.c
+++ b/gcc/genmatch.c
@@ -388,7 +388,7 @@ id_base::id_base (id_kind kind_, const char *id_, int nargs_)
   kind = kind_;
   id = id_;
   nargs = nargs_;
-  hashval = htab_hash_string (id);
+  hashval = hash_string (id);
 }
 
 /* Identifier that maps to a tree code.  */
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 6db9ed475f4..d1b2709c108 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -192,7 +192,7 @@ static hashval_t
 hash_mode (const void *p)
 {
   const struct mode_data *m = (const struct mode_data *)p;
-  return htab_hash_string (m->name);
+  return hash_string (m->name);
 }
 
 static int
diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 1d76af7ce5c..978d8e955a3 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -2434,7 +2434,7 @@ gen_mnemonic_attr (void)
   if (!mnemonic_attr)
     return;
 
-  mnemonic_htab = htab_create_alloc (MNEMONIC_HTAB_SIZE, htab_hash_string,
+  mnemonic_htab = htab_create_alloc (MNEMONIC_HTAB_SIZE, hash_string_vptr,
 				     htab_eq_string, 0, xcalloc, free);
 
   for (elem = define_insn_queue; elem; elem = elem->next)
@@ -2492,7 +2492,7 @@ check_define_attr_duplicates ()
   char * attr_name;
   void **slot;
 
-  attr_htab = htab_create (500, htab_hash_string, htab_eq_string, NULL);
+  attr_htab = htab_create (500, hash_string_vptr, htab_eq_string, NULL);
 
   for (elem = define_attr_queue; elem; elem = elem->next)
     {
@@ -2790,7 +2790,7 @@ static struct pred_data **last_predicate = &first_predicate;
 static hashval_t
 hash_struct_pred_data (const void *ptr)
 {
-  return htab_hash_string (((const struct pred_data *)ptr)->name);
+  return hash_string (((const struct pred_data *)ptr)->name);
 }
 
 static int
diff --git a/gcc/gentarget-def.c b/gcc/gentarget-def.c
index 71f3cb95296..e19e3bde6ab 100644
--- a/gcc/gentarget-def.c
+++ b/gcc/gentarget-def.c
@@ -40,7 +40,7 @@ struct insn_hasher : nofree_ptr_hash <rtx_def>
 hashval_t
 insn_hasher::hash (rtx x)
 {
-  return htab_hash_string (XSTR (x, 0));
+  return hash_string (XSTR (x, 0));
 }
 
 bool
@@ -155,7 +155,7 @@ def_target_insn (const char *name, const char *prototype)
   suffix[i] = 0;
 
   /* See whether we have an implementation of this pattern.  */
-  hashval_t hash = htab_hash_string (name);
+  hashval_t hash = hash_string (name);
   int truth = 0;
   const char *have_name = name;
   if (rtx insn = insns->find_with_hash (name, hash))
@@ -270,7 +270,7 @@ add_insn (md_rtx_info *info)
   if (name[0] == 0 || name[0] == '*')
     return;
 
-  hashval_t hash = htab_hash_string (name);
+  hashval_t hash = hash_string (name);
   rtx *slot = insns->find_slot_with_hash (name, hash, INSERT);
   if (*slot)
     error_at (info->loc, "duplicate definition of '%s'", name);
diff --git a/gcc/godump.c b/gcc/godump.c
index baf21e27eec..52e634f0cce 100644
--- a/gcc/godump.c
+++ b/gcc/godump.c
@@ -85,7 +85,7 @@ static hashval_t
 macro_hash_hashval (const void *val)
 {
   const struct macro_hash_value *mhval = (const struct macro_hash_value *) val;
-  return htab_hash_string (mhval->name);
+  return hash_string (mhval->name);
 }
 
 /* Compare values in the macro hash table for equality.  */
@@ -158,7 +158,7 @@ go_define (unsigned int lineno, const char *buffer)
   mhval->name = copy;
   mhval->value = NULL;
 
-  hashval = htab_hash_string (copy);
+  hashval = hash_string (copy);
   slot = htab_find_slot_with_hash (macro_hash, mhval, hashval, NO_INSERT);
 
   /* For simplicity, we force all names to be hidden by adding an
@@ -1377,11 +1377,11 @@ go_finish (const char *filename)
 
   real_debug_hooks->finish (filename);
 
-  container.type_hash = htab_create (100, htab_hash_string,
+  container.type_hash = htab_create (100, hash_string_vptr,
                                      string_hash_eq, NULL);
-  container.invalid_hash = htab_create (10, htab_hash_string,
+  container.invalid_hash = htab_create (10, hash_string_vptr,
 					string_hash_eq, NULL);
-  container.keyword_hash = htab_create (50, htab_hash_string,
+  container.keyword_hash = htab_create (50, hash_string_vptr,
                                         string_hash_eq, NULL);
   obstack_init (&container.type_obstack);
 
diff --git a/gcc/hash-traits.h b/gcc/hash-traits.h
index 6058cfc1d07..011050df1f7 100644
--- a/gcc/hash-traits.h
+++ b/gcc/hash-traits.h
@@ -212,7 +212,7 @@ struct string_hash : pointer_hash <const char>
 inline hashval_t
 string_hash::hash (const char *id)
 {
-  return htab_hash_string (id);
+  return hash_string (id);
 }
 
 inline bool
diff --git a/gcc/lra.c b/gcc/lra.c
index aa768fb2a23..78c09f70614 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -1755,7 +1755,7 @@ lra_rtx_hash (rtx x)
 
 	case 'S':
 	case 's':
-	  val += htab_hash_string (XSTR (x, i));
+	  val += hash_string (XSTR (x, i));
 	  break;
 
 	case 'u':
diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
index f4d340ff5a3..890d9035148 100644
--- a/gcc/lto-section-in.c
+++ b/gcc/lto-section-in.c
@@ -283,7 +283,7 @@ static hashval_t
 hash_name (const void *p)
 {
   const struct lto_renaming_slot *ds = (const struct lto_renaming_slot *) p;
-  return (hashval_t) htab_hash_string (ds->new_name);
+  return hash_string (ds->new_name);
 }
 
 /* Returns nonzero if P1 and P2 are equal.  */
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index 2d52d4a8f4e..833db70c0c5 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -70,7 +70,7 @@ static hashval_t
 hash_name (const void *p)
 {
   const struct lto_section_slot *ds = (const struct lto_section_slot *) p;
-  return (hashval_t) htab_hash_string (ds->name);
+  return hash_string (ds->name);
 }
 
 
diff --git a/gcc/plugin.c b/gcc/plugin.c
index 1a621029bf0..9191a0a2281 100644
--- a/gcc/plugin.c
+++ b/gcc/plugin.c
@@ -73,7 +73,7 @@ struct event_hasher : nofree_ptr_hash <const char *>
 inline hashval_t
 event_hasher::hash (const char **v)
 {
-  return htab_hash_string (*v);
+  return hash_string (*v);
 }
 
 /* Helper function for the event hash table that compares the name of an
@@ -131,7 +131,7 @@ static hashval_t
 htab_hash_plugin (const PTR p)
 {
   const struct plugin_name_args *plugin = (const struct plugin_name_args *) p;
-  return htab_hash_string (plugin->base_name);
+  return hash_string (plugin->base_name);
  }
 
 /* Helper function for the hash table that compares the base_name of the
@@ -222,7 +222,7 @@ add_new_plugin (const char* plugin_name)
                                         NULL);
 
   slot = htab_find_slot_with_hash (plugin_name_args_tab, base_name,
-				   htab_hash_string (base_name), INSERT);
+				   hash_string (base_name), INSERT);
 
   /* If the same plugin (name) has been specified earlier, either emit an
      error or a warning message depending on if they have identical full
@@ -310,7 +310,7 @@ parse_plugin_arg_opt (const char *arg)
      command-line.  */
   if (plugin_name_args_tab
       && ((slot = htab_find_slot_with_hash (plugin_name_args_tab, name,
-					    htab_hash_string (name), NO_INSERT))
+					    hash_string (name), NO_INSERT))
           != NULL))
     {
       struct plugin_name_args *plugin = (struct plugin_name_args *) *slot;
@@ -367,7 +367,7 @@ static void
 register_plugin_info (const char* name, struct plugin_info *info)
 {
   void **slot = htab_find_slot_with_hash (plugin_name_args_tab, name,
-					  htab_hash_string (name), NO_INSERT);
+					  hash_string (name), NO_INSERT);
   struct plugin_name_args *plugin;
 
   if (slot == NULL)
@@ -754,7 +754,7 @@ init_one_plugin (void **slot, void * ARG_UNUSED (info))
   if (!ok)
     {
       htab_remove_elt_with_hash (plugin_name_args_tab, plugin->base_name,
-				 htab_hash_string (plugin->base_name));
+				 hash_string (plugin->base_name));
       XDELETE (plugin);
     }
   return 1;
diff --git a/gcc/read-md.c b/gcc/read-md.c
index 174e471a8db..2020d6431a1 100644
--- a/gcc/read-md.c
+++ b/gcc/read-md.c
@@ -65,7 +65,7 @@ md_reader *md_reader_ptr;
 hashval_t
 leading_string_hash (const void *def)
 {
-  return htab_hash_string (*(const char *const *) def);
+  return hash_string (*(const char *const *) def);
 }
 
 /* Given two objects that start with char * name fields, return true if
diff --git a/gcc/read-rtl.c b/gcc/read-rtl.c
index d698dd4af4d..1921f5ad441 100644
--- a/gcc/read-rtl.c
+++ b/gcc/read-rtl.c
@@ -588,7 +588,7 @@ static hashval_t
 overloaded_name_hash (const void *uncast_oname)
 {
   const overloaded_name *oname = (const overloaded_name *) uncast_oname;
-  return htab_hash_string (oname->name);
+  return hash_string (oname->name);
 }
 
 /* Return true if two overloaded_names are similar enough to share
diff --git a/gcc/statistics.c b/gcc/statistics.c
index 87b6312c944..56d5804e584 100644
--- a/gcc/statistics.c
+++ b/gcc/statistics.c
@@ -56,7 +56,7 @@ struct stats_counter_hasher : pointer_hash <statistics_counter>
 inline hashval_t
 stats_counter_hasher::hash (const statistics_counter *c)
 {
-  return htab_hash_string (c->id) + c->val;
+  return hash_string (c->id) + c->val;
 }
 
 /* Compare two statistic counters by their string IDs.  */
diff --git a/gcc/symtab.c b/gcc/symtab.c
index baa825598b3..934d0982aab 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -90,10 +90,10 @@ symbol_table::decl_assembler_name_hash (const_tree asmname)
       else if (strncmp (decl_str, user_label_prefix, ulp_len) == 0)
 	decl_str += ulp_len;
 
-      return htab_hash_string (decl_str);
+      return hash_string (decl_str);
     }
 
-  return htab_hash_string (IDENTIFIER_POINTER (asmname));
+  return hash_string (IDENTIFIER_POINTER (asmname));
 }
 
 /* Return true if assembler names NAME1 and NAME2 leads to the same symbol
@@ -357,7 +357,7 @@ symbol_table::change_decl_assembler_name (tree decl, tree name)
 hashval_t
 section_name_hasher::hash (section_hash_entry *n)
 {
-  return htab_hash_string (n->name);
+  return hash_string (n->name);
 }
 
 /* Return true if section P1 name equals to P2.  */
@@ -1502,7 +1502,7 @@ symtab_node::set_section_for_node (const char *section)
       x_section->ref_count--;
       if (!x_section->ref_count)
 	{
-	  hashval_t hash = htab_hash_string (x_section->name);
+	  hashval_t hash = hash_string (x_section->name);
 	  slot = symtab->section_hash->find_slot_with_hash (x_section->name,
 							    hash, INSERT);
 	  ggc_free (x_section);
@@ -1518,7 +1518,7 @@ symtab_node::set_section_for_node (const char *section)
   if (!symtab->section_hash)
     symtab->section_hash = hash_table<section_name_hasher>::create_ggc (10);
   slot = symtab->section_hash->find_slot_with_hash (section,
-						    htab_hash_string (section),
+						    hash_string (section),
 						    INSERT);
   if (*slot)
     x_section = (section_hash_entry *)*slot;
diff --git a/gcc/system.h b/gcc/system.h
index 100feb567c9..37f3cfc4b03 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -879,6 +879,30 @@ extern void fancy_abort (const char *, int, const char *)
 #undef strerror
  #pragma GCC poison strerror
 
+#pragma GCC poison htab_hash_string
+
+/* Hash STR as a null-terminated string.  */
+
+static inline hashval_t
+hash_string (const char *str)
+{
+  hashval_t r = 0;
+  unsigned char c;
+
+  while ((c = *str++) != 0)
+    r = r * 67 + c - 113;
+
+  return r;
+}
+
+/* Hash P as a null-terminated string.  */
+
+static inline hashval_t
+hash_string_vptr (const void *p)
+{
+  return hash_string ((const char *)p);
+}
+
 /* loc_t is defined on some systems and too inviting for some
    programmers to avoid.  */
 #undef loc_t
diff --git a/gcc/tlink.c b/gcc/tlink.c
index ec20bd2fa0f..de27e7354c4 100644
--- a/gcc/tlink.c
+++ b/gcc/tlink.c
@@ -91,7 +91,7 @@ static hashval_t
 hash_string_hash (const void *s_p)
 {
   const char *const *s = (const char *const *) s_p;
-  return (*htab_hash_string) (*s);
+  return hash_string (*s);
 }
 
 static htab_t symbol_table;
@@ -122,7 +122,7 @@ symbol_hash_lookup (const char *string, int create)
 {
   void **e;
   e = htab_find_slot_with_hash (symbol_table, string,
-				(*htab_hash_string) (string),
+				hash_string (string),
 				create ? INSERT : NO_INSERT);
   if (e == NULL)
     return NULL;
@@ -144,7 +144,7 @@ file_hash_lookup (const char *string)
 {
   void **e;
   e = htab_find_slot_with_hash (file_table, string,
-				(*htab_hash_string) (string),
+				hash_string (string),
 				INSERT);
   if (*e == NULL)
     {
@@ -164,7 +164,7 @@ demangled_hash_lookup (const char *string, int create)
 {
   void **e;
   e = htab_find_slot_with_hash (demangled_table, string,
-				(*htab_hash_string) (string),
+				hash_string (string),
 				create ? INSERT : NO_INSERT);
   if (e == NULL)
     return NULL;
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 545e13fef6a..3fbc25e8062 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -215,7 +215,7 @@ section_hasher::equal (section *old, const char *new_name)
 hashval_t
 section_hasher::hash (section *old)
 {
-  return htab_hash_string (old->named.name);
+  return hash_string (old->named.name);
 }
 
 /* Return a hash value for section SECT.  */
@@ -224,7 +224,7 @@ static hashval_t
 hash_section (section *sect)
 {
   if (sect->common.flags & SECTION_NAMED)
-    return htab_hash_string (sect->named.name);
+    return hash_string (sect->named.name);
   return sect->common.flags & ~SECTION_DECLARED;
 }
 
@@ -282,7 +282,7 @@ get_section (const char *name, unsigned int flags, tree decl)
 {
   section *sect, **slot;
 
-  slot = section_htab->find_slot_with_hash (name, htab_hash_string (name),
+  slot = section_htab->find_slot_with_hash (name, hash_string (name),
 					    INSERT);
   flags |= SECTION_NAMED;
   if (*slot == NULL)
@@ -3716,7 +3716,7 @@ const_rtx_hash_1 (const_rtx x)
       break;
 
     case SYMBOL_REF:
-      h ^= htab_hash_string (XSTR (x, 0));
+      h ^= hash_string (XSTR (x, 0));
       break;
 
     case LABEL_REF:
diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
index 164ed3bf6a2..f6a82e33af1 100644
--- a/libcc1/libcc1plugin.cc
+++ b/libcc1/libcc1plugin.cc
@@ -136,7 +136,7 @@ struct string_hasher : nofree_ptr_hash<const char>
 {
   static inline hashval_t hash (const char *s)
   {
-    return htab_hash_string (s);
+    return hash_string (s);
   }
 
   static inline bool equal (const char *p1, const char *p2)
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index 10341477c56..af8886ac8a7 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -143,7 +143,7 @@ struct string_hasher : nofree_ptr_hash<const char>
 {
   static inline hashval_t hash (const char *s)
   {
-    return htab_hash_string (s);
+    return hash_string (s);
   }
 
   static inline bool equal (const char *p1, const char *p2)
-- 
2.19.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-02  9:25     ` Martin Liška
@ 2018-11-05 18:37       ` Michael Matz
  2018-11-07  8:54         ` Martin Liška
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Matz @ 2018-11-05 18:37 UTC (permalink / raw)
  To: Martin Liška; +Cc: Richard Biener, GCC Patches

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

Hi,

On Fri, 2 Nov 2018, Martin Liška wrote:

> V2 of the patch.
> 
> Thoughts?

Whereever the new function belongs it certainly isn't system.h.  Also the 
definition in a header seems excessive.  Sure, it enables inlining of it, 
but that seems premature optimization.  It contains a loop, and inlining 
anything with loops that aren't very likely to loop just once or never 
just blows code for no gain.  Also as the function is leaf there won't be 
any second-order effect from inlining.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-05 18:37       ` Michael Matz
@ 2018-11-07  8:54         ` Martin Liška
  2018-11-08 13:15           ` Michael Matz
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2018-11-07  8:54 UTC (permalink / raw)
  To: Michael Matz; +Cc: Richard Biener, GCC Patches

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

On 11/5/18 7:37 PM, Michael Matz wrote:
> Hi,
> 
> On Fri, 2 Nov 2018, Martin Liška wrote:
> 
>> V2 of the patch.
>>
>> Thoughts?
> 

Hi.

> Whereever the new function belongs it certainly isn't system.h.  Also the 
> definition in a header seems excessive.  Sure, it enables inlining of it, 
> but that seems premature optimization.  It contains a loop, and inlining 
> anything with loops that aren't very likely to loop just once or never 
> just blows code for no gain.  Also as the function is leaf there won't be 
> any second-order effect from inlining.

Ok, works for me. As you know my main motivation was to provide stronger type
declaration that can be used for 'const char *'. So what about providing 2 wrappers
and poisoning the implementation?

Martin

> 
> 
> Ciao,
> Michael.
> 


[-- Attachment #2: hash.patch --]
[-- Type: text/x-patch, Size: 693 bytes --]

diff --git a/gcc/system.h b/gcc/system.h
index 100feb567c9..fb781645350 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -879,6 +879,23 @@ extern void fancy_abort (const char *, int, const char *)
 #undef strerror
  #pragma GCC poison strerror
 
+/* Define new hash string wappers that will allow to poison
+   the implementation.  */
+
+static inline hashval_t
+hash_string_vptr (const void *p)
+{
+  return htab_hash_string (p);
+}
+
+static inline hashval_t
+hash_string (const char *str)
+{
+  return htab_hash_string ((const char *)str);
+}
+
+#pragma GCC poison htab_hash_string
+
 /* loc_t is defined on some systems and too inviting for some
    programmers to avoid.  */
 #undef loc_t

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-07  8:54         ` Martin Liška
@ 2018-11-08 13:15           ` Michael Matz
  2018-11-08 13:41             ` Martin Liška
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Matz @ 2018-11-08 13:15 UTC (permalink / raw)
  To: Martin Liška; +Cc: Richard Biener, GCC Patches

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

Hi,

On Wed, 7 Nov 2018, Martin Liška wrote:

> > Whereever the new function belongs it certainly isn't system.h.  Also 
> > the definition in a header seems excessive.  Sure, it enables inlining 
> > of it, but that seems premature optimization.  It contains a loop, and 
> > inlining anything with loops that aren't very likely to loop just once 
> > or never just blows code for no gain.  Also as the function is leaf 
> > there won't be any second-order effect from inlining.
> 
> Ok, works for me. As you know my main motivation was to provide stronger 
> type declaration that can be used for 'const char *'. So what about 
> providing 2 wrappers and poisoning the implementation?

That seems better.  But still, why declare this in system.h?  It seems 
hash-table.h seems more appropriate.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-08 13:15           ` Michael Matz
@ 2018-11-08 13:41             ` Martin Liška
  2018-11-09 13:28               ` Michael Matz
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2018-11-08 13:41 UTC (permalink / raw)
  To: Michael Matz; +Cc: Richard Biener, GCC Patches

On 11/8/18 2:15 PM, Michael Matz wrote:
> Hi,
> 
> On Wed, 7 Nov 2018, Martin Liška wrote:
> 
>>> Whereever the new function belongs it certainly isn't system.h.  Also 
>>> the definition in a header seems excessive.  Sure, it enables inlining 
>>> of it, but that seems premature optimization.  It contains a loop, and 
>>> inlining anything with loops that aren't very likely to loop just once 
>>> or never just blows code for no gain.  Also as the function is leaf 
>>> there won't be any second-order effect from inlining.
>>
>> Ok, works for me. As you know my main motivation was to provide stronger 
>> type declaration that can be used for 'const char *'. So what about 
>> providing 2 wrappers and poisoning the implementation?

Hi.

> 
> That seems better.  But still, why declare this in system.h?  It seems 
> hash-table.h seems more appropriate.

I need to declare it before I'll poison it. As system.h is included very early,
one can guarantee that there will be no usage before the poisoning happens.

Martin

> 
> 
> Ciao,
> Michael.
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-08 13:41             ` Martin Liška
@ 2018-11-09 13:28               ` Michael Matz
  2018-11-12  9:35                 ` Martin Liška
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Matz @ 2018-11-09 13:28 UTC (permalink / raw)
  To: Martin Liška; +Cc: Richard Biener, GCC Patches

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

Hi,

On Thu, 8 Nov 2018, Martin Liška wrote:

> > That seems better.  But still, why declare this in system.h?  It seems 
> > hash-table.h seems more appropriate.
> 
> I need to declare it before I'll poison it. As system.h is included very 
> early, one can guarantee that there will be no usage before the 
> poisoning happens.

Yes but it's also included everywhere, so adding anything to it comes at a 
cost, and conceptually it simply doesn't belong there.

There's no fundamental reason why we can't poison identifiers in other 
headers.  Indeed we do in vec.h.  So move the whole thing including 
poisoning to hash-table.h?


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-09 13:28               ` Michael Matz
@ 2018-11-12  9:35                 ` Martin Liška
  2018-11-12 13:43                   ` Michael Matz
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Liška @ 2018-11-12  9:35 UTC (permalink / raw)
  To: Michael Matz; +Cc: Richard Biener, GCC Patches

On 11/9/18 2:28 PM, Michael Matz wrote:
> Hi,
> 
> On Thu, 8 Nov 2018, Martin Liška wrote:
> 
>>> That seems better.  But still, why declare this in system.h?  It seems 
>>> hash-table.h seems more appropriate.
>>
>> I need to declare it before I'll poison it. As system.h is included very 
>> early, one can guarantee that there will be no usage before the 
>> poisoning happens.
> 
> Yes but it's also included everywhere, so adding anything to it comes at a 
> cost, and conceptually it simply doesn't belong there.

Agree.

> 
> There's no fundamental reason why we can't poison identifiers in other 
> headers.  Indeed we do in vec.h.  So move the whole thing including 
> poisoning to hash-table.h?

That's not feasible as gcc/gcc/genhooks.c files use the function and
we don't want to include hash-table.h in the generator files.
So second candidate can be gcc/hash-traits.h, but it's also not working:
/home/marxin/Programming/gcc/gcc/hash-traits.h:270:17: error: ‘gt_pointer_operator’ has not been declared
   pch_nx (T &p, gt_pointer_operator op, void *cookie)
                 ^~~~~~~~~~~~~~~~~~~

so we should eventually come up with "hash.h" and include it in many places as there's following usage
in hash-traits.h:

   212  inline hashval_t
   213  string_hash::hash (const char *id)
   214  {
   215    return hash_string (id);
   216  }

So it's question whether it worth doing that?

Martin

> 
> 
> Ciao,
> Michael.
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible.
  2018-11-12  9:35                 ` Martin Liška
@ 2018-11-12 13:43                   ` Michael Matz
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Matz @ 2018-11-12 13:43 UTC (permalink / raw)
  To: Martin Liška; +Cc: Richard Biener, GCC Patches

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

Hi,

On Mon, 12 Nov 2018, Martin Liška wrote:

> > There's no fundamental reason why we can't poison identifiers in other 
> > headers.  Indeed we do in vec.h.  So move the whole thing including 
> > poisoning to hash-table.h?
> 
> That's not feasible as gcc/gcc/genhooks.c files use the function and
> we don't want to include hash-table.h in the generator files.

gencfn-macros.c:#include "hash-table.h"
genmatch.c:#include "hash-table.h"
gentarget-def.c:#include "hash-table.h"

So there's precedent.  The other solution would be to ignore genhooks.c 
(i.e. let it continue using the non-typesafe variant), I'm not very 
worried about wrong uses creeping in there.  It had like one material 
change in the last seven years.

I think I prefer the latter (ignoring the problem).

> So it's question whether it worth doing that?

Jumping through hoops for generator files seems useless.  But the general 
idea for your type-checking hashers for the compiler proper does seem 
useful.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-12 13:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 17:32 [PATCH] Come up with htab_hash_string_vptr and use string-specific if possible Martin Liška
2018-11-02  8:03 ` Richard Biener
2018-11-02  8:10   ` Martin Liška
2018-11-02  9:25     ` Martin Liška
2018-11-05 18:37       ` Michael Matz
2018-11-07  8:54         ` Martin Liška
2018-11-08 13:15           ` Michael Matz
2018-11-08 13:41             ` Martin Liška
2018-11-09 13:28               ` Michael Matz
2018-11-12  9:35                 ` Martin Liška
2018-11-12 13:43                   ` Michael Matz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).