Patchwork [1/9] Improve pointer hash function to include all bits

login
register
mail settings
Submitter David Edelsohn
Date May 4, 2013, 8:41 p.m.
Message ID <CAGWvny=-baLJY+zA0fVoUN_H45Mje4sGK8DU5T11cOpNe=Tugw@mail.gmail.com>
Download mbox | patch
Permalink /patch/241475/
State New
Headers show

Comments

David Edelsohn - May 4, 2013, 8:41 p.m.
The revised version of hash_pointer() produces warnings when compiled
on a 32 bit host because it shift a 32 bit variable by 32 bits.

+  intptr_t v = (intptr_t)p;
...
+      b += v & 0xffffffff;

The code is not executed when intptr_t is 32 bits, but it still is
present in the function, causing a warning.

The appended, revised implementation was designed by a blue ribbon commission.

Bootstrapped on powerpc-ibm-aix7.1.0.0.

Okay for trunk?

2013-05-04  David Edelsohn  <dje.gcc@gmail.com>
                  Peter Bergner  <bergner@vnet.ibm.com>
                  Segher Boessenkool  <segher@kernel.crashing.org>
                  Jakub Jelinek  <jakub@redhat.com>
Andi Kleen - May 5, 2013, 5:10 p.m.
> +  a += v >> (sizeof (intptr_t) * CHAR_BIT / 2);
> +  b += v & (((intptr_t) 1 << (sizeof (intptr_t) * CHAR_BIT / 2)) - 1);

It would be far easier to read if you added a BITS_PER_POINTER define
somewhere. Other than that it looks good to me, but I cannot approve.
-Andi
Ian Taylor - May 6, 2013, 2:25 a.m.
On Sat, May 4, 2013 at 1:41 PM, David Edelsohn <dje.gcc@gmail.com> wrote:

> 2013-05-04  David Edelsohn  <dje.gcc@gmail.com>
>                   Peter Bergner  <bergner@vnet.ibm.com>
>                   Segher Boessenkool  <segher@kernel.crashing.org>
>                   Jakub Jelinek  <jakub@redhat.com>

This is OK (the actual ChangeLog entry was omitted).

Thanks.

Ian

Patch

Index: hashtab.c
===================================================================
--- hashtab.c   (revision 198587)
+++ hashtab.c   (working copy)
@@ -990,17 +990,8 @@ 
   unsigned a, b, c;

   a = b = 0x9e3779b9;
-  if (sizeof (intptr_t) == 4)
-    {
-      /* Mix as 16bit for now */
-      a += v >> 16;
-      b += v & 0xffff;
-    }
-  else
-    {
-      a += v >> 32;
-      b += v & 0xffffffff;
-    }
+  a += v >> (sizeof (intptr_t) * CHAR_BIT / 2);
+  b += v & (((intptr_t) 1 << (sizeof (intptr_t) * CHAR_BIT / 2)) - 1);
   c = 0x42135234;
   mix (a, b, c);
   return c;