[llvm-commits] [llvm] r150890 - in /llvm/trunk: include/llvm/ADT/Hashing.h lib/Support/CMakeLists.txt lib/Support/Hashing.cpp unittests/ADT/HashingTest.cpp unittests/CMakeLists.txt

Talin viridia at gmail.com
Mon Feb 20 22:43:03 PST 2012


On Mon, Feb 20, 2012 at 10:38 AM, Jay Foad <jay.foad at gmail.com> wrote:

> On 18 February 2012 21:00, Talin <viridia at gmail.com> wrote:
> > Added: llvm/trunk/include/llvm/ADT/Hashing.h
>
> Nitpick: wouldn't this be better in ../Support ?
>

I'm not sure what the boundary between ADT and Support is - I put it in ADT
because containers use it. However, if someone moves it to Support I won't
complain.

>
> > +  /// Add a float
> > +  GeneralHash& add(float Data) {
> > +    union {
> > +      float D; uint32_t I;
> > +    };
> > +    D = Data;
> > +    addInt(I);
> > +    return *this;
> > +  }
> > +
> > +  /// Add a double
> > +  GeneralHash& add(double Data) {
> > +    union {
> > +      double D; uint64_t I;
> > +    };
> > +    D = Data;
> > +    addInt(I);
> > +    return *this;
> > +  }
>
> IMO it would be better not to implement these at all until someone
> needs them, and decides what to do about the +/- 0 problem. (But
> that's just another nitpick!)
>
> For uniquing of ConstantFPs, I think you do want to distinguish between
+/- 0.


>  > +// Add a possibly unaligned sequence of bytes.
> > +void GeneralHash::addUnaligned(const uint8_t *I, const uint8_t *E) {
> > +  ptrdiff_t Length = E - I;
> > +  if (uintptr_t(I) & 3 == 0) {
> > +    while (Length > 3) {
> > +      mix(*reinterpret_cast<const uint32_t *>(I));
> > +      I += 4;
> > +      Length -= 4;
> > +    }
> > +  } else {
> > +    while (Length > 3) {
> > +      mix(
> > +        uint32_t(I[0]) +
> > +        (uint32_t(I[1]) << 8) +
> > +        (uint32_t(I[2]) << 16) +
> > +        (uint32_t(I[3]) << 24));
> > +      I += 4;
> > +      Length -= 4;
> > +    }
> > +  }
>
> I think there's a serious problem here on big-endian hosts, because
> identical arrays of bytes will hash to different values depending on
> whether they happen to start on a 4-byte boundary or not.
>

Good catch. Shouldn't be hard to fix I would think.

>
> Thanks for working on this!
> Jay.
>



-- 
-- Talin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120220/5aafe0f2/attachment.html>


More information about the llvm-commits mailing list