From 37b04503db54a33562136b04755ae8a3b7904f25 Mon Sep 17 00:00:00 2001 From: greg Date: Sun, 19 Jul 2020 17:51:13 -0400 Subject: [PATCH] cleanup some test failures. --- parallel_hashmap/phmap.h | 22 ++++++++++++++-------- parallel_hashmap/phmap_bits.h | 18 +++++++++--------- parallel_hashmap/phmap_config.h | 2 ++ tests/flat_hash_map_test.cc | 7 +++++-- tests/raw_hash_set_test.cc | 13 +++++-------- tests/unordered_map_modifiers_test.h | 8 ++++++++ 6 files changed, 43 insertions(+), 27 deletions(-) diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index 47cf42a..681cb30 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -137,18 +137,19 @@ constexpr bool IsNoThrowSwappable() { // -------------------------------------------------------------------------- template int TrailingZeros(T x) { - return sizeof(T) == 8 ? base_internal::CountTrailingZerosNonZero64( - static_cast(x)) - : base_internal::CountTrailingZerosNonZero32( - static_cast(x)); + PHMAP_IF_CONSTEXPR(sizeof(T) == 8) + return base_internal::CountTrailingZerosNonZero64(static_cast(x)); + else + return base_internal::CountTrailingZerosNonZero32(static_cast(x)); } // -------------------------------------------------------------------------- template int LeadingZeros(T x) { - return sizeof(T) == 8 - ? base_internal::CountLeadingZeros64(static_cast(x)) - : base_internal::CountLeadingZeros32(static_cast(x)); + PHMAP_IF_CONSTEXPR(sizeof(T) == 8) + return base_internal::CountLeadingZeros64(static_cast(x)); + else + return base_internal::CountLeadingZeros32(static_cast(x)); } // -------------------------------------------------------------------------- @@ -1432,7 +1433,12 @@ public: // This overload is necessary because otherwise erase(const K&) would be // a better match if non-const iterator is passed as an argument. - iterator erase(iterator it) { _erase(it++); return it; } + iterator erase(iterator it) { + auto res = it; + ++res; + _erase(it); + return res; + } iterator erase(const_iterator first, const_iterator last) { while (first != last) { diff --git a/parallel_hashmap/phmap_bits.h b/parallel_hashmap/phmap_bits.h index 7933d8c..6b765ff 100644 --- a/parallel_hashmap/phmap_bits.h +++ b/parallel_hashmap/phmap_bits.h @@ -287,7 +287,7 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros64(uint64_t n) { return (int)(63 - result); } return 64; -#elif defined(_MSC_VER) +#elif defined(_MSC_VER) && !defined(__clang__) // MSVC does not have __buitin_clzll. Compose two calls to _BitScanReverse unsigned long result = 0; // NOLINT(runtime/int) if ((n >> 32) && _BitScanReverse(&result, (unsigned long)(n >> 32))) { @@ -297,7 +297,7 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros64(uint64_t n) { return 63 - result; } return 64; -#elif defined(__GNUC__) +#elif defined(__GNUC__) || defined(__clang__) // Use __builtin_clzll, which uses the following instructions: // x86: bsr // ARM64: clz @@ -324,13 +324,13 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros32Slow(uint64_t n) { } PHMAP_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros32(uint32_t n) { -#if defined(_MSC_VER) +#if defined(_MSC_VER) && !defined(__clang__) unsigned long result = 0; // NOLINT(runtime/int) if (_BitScanReverse(&result, n)) { return (int)(31 - result); } return 32; -#elif defined(__GNUC__) +#elif defined(__GNUC__) || defined(__clang__) // Use __builtin_clz, which uses the following instructions: // x86: bsr // ARM64: clz @@ -361,11 +361,11 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero64Slow(uint64_t n) } PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero64(uint64_t n) { -#if defined(_MSC_VER) && defined(_M_X64) +#if defined(_MSC_VER) && !defined(__clang__) && defined(_M_X64) unsigned long result = 0; // NOLINT(runtime/int) _BitScanForward64(&result, n); return (int)result; -#elif defined(_MSC_VER) +#elif defined(_MSC_VER) && !defined(__clang__) unsigned long result = 0; // NOLINT(runtime/int) if (static_cast(n) == 0) { _BitScanForward(&result, (unsigned long)(n >> 32)); @@ -373,7 +373,7 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero64(uint64_t n) { } _BitScanForward(&result, (unsigned long)n); return result; -#elif defined(__GNUC__) +#elif defined(__GNUC__) || defined(__clang__) static_assert(sizeof(unsigned long long) == sizeof(n), // NOLINT(runtime/int) "__builtin_ctzll does not take 64-bit arg"); return __builtin_ctzll(n); @@ -394,11 +394,11 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero32Slow(uint32_t n) } PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero32(uint32_t n) { -#if defined(_MSC_VER) +#if defined(_MSC_VER) && !defined(__clang__) unsigned long result = 0; // NOLINT(runtime/int) _BitScanForward(&result, n); return (int)result; -#elif defined(__GNUC__) +#elif defined(__GNUC__) || defined(__clang__) static_assert(sizeof(int) == sizeof(n), "__builtin_ctz does not take 32-bit arg"); return __builtin_ctz(n); diff --git a/parallel_hashmap/phmap_config.h b/parallel_hashmap/phmap_config.h index 5e527cc..a36829a 100644 --- a/parallel_hashmap/phmap_config.h +++ b/parallel_hashmap/phmap_config.h @@ -126,6 +126,8 @@ #define PHMAP_HAVE_CC17 0 #endif +#define PHMAP_BRANCHLESS 1 + // ---------------------------------------------------------------- // Checks whether `std::is_trivially_destructible` is supported. // ---------------------------------------------------------------- diff --git a/tests/flat_hash_map_test.cc b/tests/flat_hash_map_test.cc index 5f0e57f..936a801 100644 --- a/tests/flat_hash_map_test.cc +++ b/tests/flat_hash_map_test.cc @@ -15,6 +15,7 @@ #ifndef THIS_HASH_MAP #define THIS_HASH_MAP flat_hash_map #define THIS_TEST_NAME FlatHashMap + #define ORIG_FLAT_HASH_MAP 1 #endif #ifndef THIS_EXTRA_TPL_PARAMS @@ -198,14 +199,16 @@ TEST(THIS_TEST_NAME, LazyKeyPattern) { m.try_emplace(LazyInt(2, &conversions), 3); EXPECT_THAT(m, UnorderedElementsAre(Pair(1, 2), Pair(2, 3))); EXPECT_EQ(conversions, 2); -#ifdef NDEBUG +#if defined(NDEBUG) && ORIG_FLAT_HASH_MAP + // for parallel maps, the reserve(3) above is not sufficient to guarantee that a submap will not resize and therefore rehash EXPECT_EQ(hashes, 3); #endif m.try_emplace(LazyInt(2, &conversions), 4); EXPECT_THAT(m, UnorderedElementsAre(Pair(1, 2), Pair(2, 3))); EXPECT_EQ(conversions, 2); -#ifdef NDEBUG +#if defined(NDEBUG) && ORIG_FLAT_HASH_MAP + // for parallel maps, the reserve(3) above is not sufficient to guarantee that a submap will not resize and therefore rehash EXPECT_EQ(hashes, 4); #endif } diff --git a/tests/raw_hash_set_test.cc b/tests/raw_hash_set_test.cc index f6fccf5..0810931 100644 --- a/tests/raw_hash_set_test.cc +++ b/tests/raw_hash_set_test.cc @@ -1112,8 +1112,7 @@ ExpectedStats XorSeedExpectedStats() { // The effective load factor is larger in non-opt mode because we insert // elements out of order. - switch (container_internal::Group::kWidth) { - case 8: + PHMAP_IF_CONSTEXPR (container_internal::Group::kWidth == 8) { if (kRandomizesInserts) { return {0.05, 1.0, @@ -1125,8 +1124,8 @@ ExpectedStats XorSeedExpectedStats() { {{0.95, 0.1}}, {{0.95, 0}, {0.99, 2}, {0.999, 4}, {0.9999, 10}}}; } - case 16: - default: + } + else { if (kRandomizesInserts) { return {0.1, 1.0, @@ -1217,8 +1216,7 @@ ExpectedStats LinearTransformExpectedStats() { // The effective load factor is larger in non-opt mode because we insert // elements out of order. - switch (container_internal::Group::kWidth) { - case 8: + PHMAP_IF_CONSTEXPR (container_internal::Group::kWidth == 8) { if (kRandomizesInserts) { return {0.1, 0.5, @@ -1230,8 +1228,7 @@ ExpectedStats LinearTransformExpectedStats() { {{0.95, 0.3}}, {{0.95, 0}, {0.99, 3}, {0.999, 15}, {0.9999, 25}}}; } - case 16: - default: + } else { if (kRandomizesInserts) { return {0.1, 0.4, diff --git a/tests/unordered_map_modifiers_test.h b/tests/unordered_map_modifiers_test.h index d26f65d..9c39b36 100644 --- a/tests/unordered_map_modifiers_test.h +++ b/tests/unordered_map_modifiers_test.h @@ -15,11 +15,19 @@ #ifndef PHMAP_CONTAINER_INTERNAL_UNORDERED_MAP_MODIFIERS_TEST_H_ #define PHMAP_CONTAINER_INTERNAL_UNORDERED_MAP_MODIFIERS_TEST_H_ +#ifdef _MSC_VER + #pragma warning(push, 0) +#endif + #include "gmock/gmock.h" #include "gtest/gtest.h" #include "hash_generator_testing.h" #include "hash_policy_testing.h" +#ifdef _MSC_VER + #pragma warning(pop) +#endif + namespace phmap { namespace container_internal {