[llvm-commits] [llvm] r151247 - in /llvm/trunk: include/llvm/ADT/Hashing.h lib/Support/Hashing.cpp

[Jay Foad]

Author: foad
Date: Thu Feb 23 03:16:04 2012
New Revision: 151247

URL: http://llvm.org/viewvc/llvm-project?rev=151247&view=rev
Log:
The implementation of GeneralHash::addBits broke C++ aliasing rules; fix
it with memcpy. This also fixes a problem on big-endian hosts, where
addUnaligned would return different results depending on the alignment
of the data.

Removed:
    llvm/trunk/lib/Support/Hashing.cpp
Modified:
    llvm/trunk/include/llvm/ADT/Hashing.h

Modified: llvm/trunk/include/llvm/ADT/Hashing.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Hashing.h?rev=151247&r1=151246&r2=151247&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/Hashing.h (original)
+++ llvm/trunk/include/llvm/ADT/Hashing.h Thu Feb 23 03:16:04 2012
@@ -19,6 +19,7 @@
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataTypes.h"
+#include <cstring>
 
 namespace llvm {
 
@@ -140,39 +141,32 @@
     mix(uint32_t(Val));
   }
 
-  template<typename T, bool isAligned>
-  struct addBitsImpl {
-    static void add(GeneralHash &Hash, const T *I, const T *E) {
-      Hash.addUnaligned(
-        reinterpret_cast<const uint8_t *>(I),
-        reinterpret_cast<const uint8_t *>(E));
+  // Add a range of bytes from I to E.
+  void addBytes(const char *I, const char *E) {
+    uint32_t Data;
+    // Note that aliasing rules forbid us from dereferencing
+    // reinterpret_cast<uint32_t *>(I) even if I happens to be suitably
+    // aligned, so we use memcpy instead.
+    for (; E - I >= ptrdiff_t(sizeof Data); I += sizeof Data) {
+      // A clever compiler should be able to turn this memcpy into a single
+      // aligned or unaligned load (depending on the alignment of the type T
+      // that was used in the call to addBits).
+      std::memcpy(&Data, I, sizeof Data);
+      mix(Data);
     }
-  };
-
-  template<typename T>
-  struct addBitsImpl<T, true> {
-    static void add(GeneralHash &Hash, const T *I, const T *E) {
-      Hash.addAligned(
-        reinterpret_cast<const uint32_t *>(I),
-        reinterpret_cast<const uint32_t *>(E));
+    if (I != E) {
+      Data = 0;
+      std::memcpy(&Data, I, E - I);
+      mix(Data);
     }
-  };
+  }
 
   // Add a range of bits from I to E.
   template<typename T>
   void addBits(const T *I, const T *E) {
-    addBitsImpl<T, AlignOf<T>::Alignment_GreaterEqual_4Bytes>::add(*this, I, E);
+    addBytes(reinterpret_cast<const char *>(I),
+             reinterpret_cast<const char *>(E));
   }
-
-  // Add a range of uint32s
-  void addAligned(const uint32_t *I, const uint32_t *E) {
-    while (I < E) {
-      mix(*I++);
-    }
-  }
-
-  // Add a possibly unaligned sequence of bytes.
-  void addUnaligned(const uint8_t *I, const uint8_t *E);
 };
 
 } // end namespace llvm

Removed: llvm/trunk/lib/Support/Hashing.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Hashing.cpp?rev=151246&view=auto
==============================================================================
--- llvm/trunk/lib/Support/Hashing.cpp (original)
+++ llvm/trunk/lib/Support/Hashing.cpp (removed)
@@ -1,46 +0,0 @@
-//===-- llvm/ADT/Hashing.cpp - Utilities for hashing ------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "llvm/ADT/Hashing.h"
-
-namespace llvm {
-
-// 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;
-    }
-  }
-
-  if (Length & 3) {
-    uint32_t Data = 0;
-    switch (Length & 3) {
-      case 3: Data |= uint32_t(I[2]) << 16;   // fall through
-      case 2: Data |= uint32_t(I[1]) << 8;    // fall through
-      case 1: Data |= uint32_t(I[0]); break;
-    }
-    mix(Data);
-  }
-}
-
-}