tint/utils: Cleanup & optimize hash utilities

Do less work for single-value hashes.
Specialize pointer hashing to improve hash quality.

Change-Id: I2f3839d15754543735728814c7f54a5e7ac81569
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/99282
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>

src/tint/fuzzers/random_generator.cc

@@ -34,10 +34,9 @@ namespace {
 /// @param size - number of elements in buffer
 /// @returns hash of the data in the buffer
 size_t HashBuffer(const uint8_t* data, const size_t size) {
-    size_t hash = 102931;
+    size_t hash = utils::Hash(size);
-    utils::HashCombine(&hash, size);
     for (size_t i = 0; i < size; i++) {
-        utils::HashCombine(&hash, data[i]);
+        hash = utils::HashCombine(hash, data[i]);
     }
     return hash;
 }

src/tint/resolver/const_eval.cc

@@ -323,7 +323,7 @@ struct Composite : Constant {
     size_t CalcHash() {
         auto h = utils::Hash(type, all_zero, any_zero);
         for (auto* el : elements) {
-            utils::HashCombine(&h, el->Hash());
+            h = utils::HashCombine(h, el->Hash());
         }
         return h;
     }

src/tint/resolver/intrinsic_table.cc

@@ -935,7 +935,7 @@ struct IntrinsicPrototype {
         inline std::size_t operator()(const IntrinsicPrototype& i) const {
             size_t hash = utils::Hash(i.parameters.Length());
             for (auto& p : i.parameters) {
-                utils::HashCombine(&hash, p.type, p.usage);
+                hash = utils::HashCombine(hash, p.type, p.usage);
             }
             return utils::Hash(hash, i.overload, i.return_type);
         }

src/tint/sem/call_target.cc

@@ -70,7 +70,7 @@ std::size_t hash<tint::sem::CallTargetSignature>::operator()(
     const tint::sem::CallTargetSignature& sig) const {
     size_t hash = tint::utils::Hash(sig.parameters.Length());
     for (auto* p : sig.parameters) {
-        tint::utils::HashCombine(&hash, p->Type(), p->Usage());
+        hash = tint::utils::HashCombine(hash, p->Type(), p->Usage());
     }
     return tint::utils::Hash(hash, sig.return_type);
 }

src/tint/transform/remove_phonies.cc

@@ -33,33 +33,7 @@ TINT_INSTANTIATE_TYPEINFO(tint::transform::RemovePhonies);
 namespace tint::transform {
 namespace {
 
-struct SinkSignature {
+using SinkSignature = std::vector<const sem::Type*>;
-    std::vector<const sem::Type*> types;
-
-    bool operator==(const SinkSignature& other) const {
-        if (types.size() != other.types.size()) {
-            return false;
-        }
-        for (size_t i = 0; i < types.size(); i++) {
-            if (types[i] != other.types[i]) {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    struct Hasher {
-        /// @param sig the CallTargetSignature to hash
-        /// @return the hash value
-        std::size_t operator()(const SinkSignature& sig) const {
-            size_t hash = tint::utils::Hash(sig.types.size());
-            for (auto* ty : sig.types) {
-                tint::utils::HashCombine(&hash, ty);
-            }
-            return hash;
-        }
-    };
-};
 
 }  // namespace
 
@@ -84,7 +58,7 @@ bool RemovePhonies::ShouldRun(const Program* program, const DataMap&) const {
 void RemovePhonies::Run(CloneContext& ctx, const DataMap&, DataMap&) const {
     auto& sem = ctx.src->Sem();
 
-    std::unordered_map<SinkSignature, Symbol, SinkSignature::Hasher> sinks;
+    std::unordered_map<SinkSignature, Symbol, utils::Hasher<SinkSignature>> sinks;
 
     for (auto* node : ctx.src->ASTNodes().Objects()) {
         Switch(
@@ -138,12 +112,12 @@ void RemovePhonies::Run(CloneContext& ctx, const DataMap&, DataMap&) const {
                     ctx.Replace(stmt, [&, side_effects] {
                         SinkSignature sig;
                         for (auto* arg : side_effects) {
-                            sig.types.push_back(sem.Get(arg)->Type()->UnwrapRef());
+                            sig.push_back(sem.Get(arg)->Type()->UnwrapRef());
                         }
                         auto sink = utils::GetOrCreate(sinks, sig, [&] {
                             auto name = ctx.dst->Symbols().New("phony_sink");
                             utils::Vector<const ast::Parameter*, 8> params;
-                            for (auto* ty : sig.types) {
+                            for (auto* ty : sig) {
                                 auto* ast_ty = CreateASTTypeFor(ctx, ty);
                                 params.Push(
                                     ctx.dst->Param("p" + std::to_string(params.Length()), ast_ty));

src/tint/utils/hash.h

@@ -48,55 +48,96 @@ struct HashCombineOffset<8> {
 
 }  // namespace detail
 
-// Forward declaration
+/// Forward declarations (see below)
 template <typename... ARGS>
-size_t Hash(const ARGS&... args);
+size_t Hash(const ARGS&... values);
 
-/// HashCombine "hashes" together an existing hash and hashable values.
+template <typename... ARGS>
+size_t HashCombine(size_t hash, const ARGS&... values);
+
+/// A STL-compatible hasher that does a more thorough job than most implementations of std::hash.
+/// Hasher has been optimized for a better quality hash at the expense of increased computation
+/// costs.
 template <typename T>
-void HashCombine(size_t* hash, const T& value) {
+struct Hasher {
-    constexpr size_t offset = detail::HashCombineOffset<sizeof(size_t)>::value();
+    /// @param value the value to hash
-    *hash ^= std::hash<T>()(value) + offset + (*hash << 6) + (*hash >> 2);
+    /// @returns a hash of the value
+    size_t operator()(const T& value) const { return std::hash<T>()(value); }
+};
+
+/// Hasher specialization for pointers
+/// std::hash<T*> typically uses a reinterpret of the pointer to a size_t.
+/// As most pointers a 4 or 16 byte aligned, this usually results in the LSBs of the hash being 0,
+/// resulting in bad hashes for hashtables. This implementation mixes up those LSBs.
+template <typename T>
+struct Hasher<T*> {
+    /// @param ptr the pointer to hash
+    /// @returns a hash of the pointer
+    size_t operator()(T* ptr) const {
+        auto hash = std::hash<T*>()(ptr);
+        return hash ^ (hash >> 4);
     }
+};
 
-/// HashCombine "hashes" together an existing hash and hashable values.
+/// Hasher specialization for std::vector
 template <typename T>
-void HashCombine(size_t* hash, const std::vector<T>& vector) {
+struct Hasher<std::vector<T>> {
-    HashCombine(hash, vector.size());
+    /// @param vector the vector to hash
+    /// @returns a hash of the vector
+    size_t operator()(const std::vector<T>& vector) const {
+        auto hash = Hash(vector.size());
         for (auto& el : vector) {
-        HashCombine(hash, el);
+            hash = HashCombine(hash, el);
         }
+        return hash;
     }
+};
 
-/// HashCombine "hashes" together an existing hash and hashable values.
+/// Hasher specialization for utils::vector
 template <typename T, size_t N>
-void HashCombine(size_t* hash, const utils::Vector<T, N>& list) {
+struct Hasher<utils::Vector<T, N>> {
-    HashCombine(hash, list.Length());
+    /// @param vector the vector to hash
-    for (auto& el : list) {
+    /// @returns a hash of the vector
-        HashCombine(hash, el);
+    size_t operator()(const utils::Vector<T, N>& vector) const {
+        auto hash = Hash(vector.Length());
+        for (auto& el : vector) {
+            hash = HashCombine(hash, el);
         }
+        return hash;
     }
+};
 
-/// HashCombine "hashes" together an existing hash and hashable values.
+/// Hasher specialization for std::tuple
 template <typename... TYPES>
-void HashCombine(size_t* hash, const std::tuple<TYPES...>& tuple) {
+struct Hasher<std::tuple<TYPES...>> {
-    HashCombine(hash, sizeof...(TYPES));
+    /// @param tuple the tuple to hash
-    HashCombine(hash, std::apply(Hash<TYPES...>, tuple));
+    /// @returns a hash of the tuple
+    size_t operator()(const std::tuple<TYPES...>& tuple) const {
+        return std::apply(Hash<TYPES...>, tuple);
     }
+};
 
-/// HashCombine "hashes" together an existing hash and hashable values.
+/// @returns a hash of the variadic list of arguments.
-template <typename T, typename... ARGS>
+///          The returned hash is dependent on the order of the arguments.
-void HashCombine(size_t* hash, const T& value, const ARGS&... args) {
-    HashCombine(hash, value);
-    HashCombine(hash, args...);
-}
-
-/// @returns a hash of the combined arguments. The returned hash is dependent on
-/// the order of the arguments.
 template <typename... ARGS>
 size_t Hash(const ARGS&... args) {
+    if constexpr (sizeof...(ARGS) == 0) {
+        return 0;
+    } else if constexpr (sizeof...(ARGS) == 1) {
+        using T = std::tuple_element_t<0, std::tuple<ARGS...>>;
+        return Hasher<T>()(args...);
+    } else {
         size_t hash = 102931;  // seed with an arbitrary prime
-    HashCombine(&hash, args...);
+        return HashCombine(hash, args...);
+    }
+}
+
+/// @returns a hash of the variadic list of arguments.
+///          The returned hash is dependent on the order of the arguments.
+template <typename... ARGS>
+size_t HashCombine(size_t hash, const ARGS&... values) {
+    constexpr size_t offset = detail::HashCombineOffset<sizeof(size_t)>::value();
+    ((hash ^= Hash(values) + offset + (hash << 6) + (hash >> 2)), ...);
     return hash;
 }