From 40119fb9c6708ff5add5a8a830a2e456eec286f8 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Fri, 2 Mar 2012 09:26:36 +0000 Subject: [PATCH] We really want to hash pairs of directly-hashable data as directly hashable data. This matters when we have pair as a key, which is quite common in DenseMap, etc. To that end, we need to detect when this is safe. The requirements on a generic std::pair are: 1) Both T and U must satisfy the existing is_hashable_data trait. Note that this includes the requirement that T and U have no internal padding bits or other bits not contributing directly to equality. 2) The alignment constraints of std::pair do not require padding between consecutive objects. 3) The alignment constraints of U and the size of T do not conspire to require padding between the first and second elements. Grow two somewhat magical traits to detect this by forming a pod structure and inspecting offset artifacts on it. Hopefully this won't cause any compilers to panic. Added and adjusted tests now that pairs, even nested pairs, are treated as just sequences of data. Thanks to Jeffrey Yasskin for helping me sort through this and reviewing the somewhat subtle traits. llvm-svn: 151883 --- llvm/include/llvm/ADT/Hashing.h | 10 ++++++++++ llvm/include/llvm/Support/type_traits.h | 18 ++++++++++++++++++ llvm/unittests/ADT/HashingTest.cpp | 12 +++++++----- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/ADT/Hashing.h b/llvm/include/llvm/ADT/Hashing.h index 06dd76a2ed78..1b0e66947b20 100644 --- a/llvm/include/llvm/ADT/Hashing.h +++ b/llvm/include/llvm/ADT/Hashing.h @@ -350,6 +350,16 @@ template struct is_hashable_data : integral_constant::value || is_pointer::value) && 64 % sizeof(T) == 0)> {}; +// Special case std::pair to detect when both types are viable and when there +// is no alignment-derived padding in the pair. This is a bit of a lie because +// std::pair isn't truly POD, but it's close enough in all reasonable +// implementations for our use case of hashing the underlying data. +template struct is_hashable_data > + : integral_constant::value && + is_hashable_data::value && + !is_alignment_padded >::value && + !is_pod_pair_padded::value)> {}; + /// \brief Helper to get the hashable data representation for a type. /// This variant is enabled when the type itself can be used. template diff --git a/llvm/include/llvm/Support/type_traits.h b/llvm/include/llvm/Support/type_traits.h index 03f85e99971a..f488a29c7af4 100644 --- a/llvm/include/llvm/Support/type_traits.h +++ b/llvm/include/llvm/Support/type_traits.h @@ -125,6 +125,24 @@ struct is_integral : is_integral_impl {}; template struct is_pointer : false_type {}; template struct is_pointer : true_type {}; +/// \brief Metafunction to compute whether a type requires alignment padding. +/// When true, an object of this type will have padding bytes inside its +/// 'sizeof' bytes. +template class is_alignment_padded { + struct pod_size_tester { T t; char c; }; +public: + enum { value = offsetof(pod_size_tester, c) != sizeof(T) }; +}; + +/// \brief Metafunction to determine whether an adjacent pair of two types will +/// require padding between them due to alignment. +template class is_pod_pair_padded { + struct pod_pair { T t; U u; }; + struct pod_char_pair { T t; char c; }; +public: + enum { value = offsetof(pod_pair, u) != offsetof(pod_char_pair, c) }; +}; + // enable_if_c - Enable/disable a template based on a metafunction template diff --git a/llvm/unittests/ADT/HashingTest.cpp b/llvm/unittests/ADT/HashingTest.cpp index a9458bb5a5a1..19a036cdf53c 100644 --- a/llvm/unittests/ADT/HashingTest.cpp +++ b/llvm/unittests/ADT/HashingTest.cpp @@ -66,11 +66,13 @@ TEST(HashingTest, HashValueBasicTest) { EXPECT_NE(hash_combine(42, 43), hash_value(std::make_pair(42ull, 43ull))); EXPECT_NE(hash_combine(42, 43), hash_value(std::make_pair(42, 43ull))); EXPECT_NE(hash_combine(42, 43), hash_value(std::make_pair(42ull, 43))); - EXPECT_EQ(hash_combine(42, hash_combine(43, hash_combine(44, 45))), - hash_value( - std::make_pair(42, std::make_pair(43, std::make_pair(44, 45))))); - EXPECT_EQ(hash_combine(42, 43), hash_value(std::make_pair(42, 43))); - EXPECT_EQ(hash_combine(42, 43), hash_value(std::make_pair(42, 43))); + + // Note that pairs are implicitly flattened to a direct sequence of data and + // hashed efficiently as a consequence. + EXPECT_EQ(hash_combine(42, 43, 44), + hash_value(std::make_pair(42, std::make_pair(43, 44)))); + EXPECT_EQ(hash_value(std::make_pair(42, std::make_pair(43, 44))), + hash_value(std::make_pair(std::make_pair(42, 43), 44))); } template T *begin(T (&arr)[N]) { return arr; }