From 7fadb0b56a7e791abdbe861594bc3f8a9893e2c2 Mon Sep 17 00:00:00 2001 From: Sam Yates <halfflat@gmail.com> Date: Wed, 11 Dec 2019 21:27:35 +0100 Subject: [PATCH] Fix support for i686 32-bit target. (#923) * Use alternative hashing of cell_member_type values for 32-bit target. * Correctly match i686 as a "-march=" style target for gcc and clang. * Correct a signed/unsigned comparison warning in test_algorithms.cpp with -Wall. * Avoid spurious SIMD unit test failure for exprelr by using expm1 instead of exp - 1 in denominator when computing expected results. --- arbor/include/arbor/common_types.hpp | 22 ++++++++++++++++++---- cmake/CompilerOptions.cmake | 2 +- test/unit/test_algorithms.cpp | 18 +++++++++--------- test/unit/test_simd.cpp | 2 +- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/arbor/include/arbor/common_types.hpp b/arbor/include/arbor/common_types.hpp index 3741f120..de9dc376 100644 --- a/arbor/include/arbor/common_types.hpp +++ b/arbor/include/arbor/common_types.hpp @@ -99,10 +99,24 @@ namespace std { template <> struct hash<arb::cell_member_type> { std::size_t operator()(const arb::cell_member_type& m) const { using namespace arb; - static_assert(sizeof(std::size_t)>sizeof(cell_gid_type), "invalid size assumptions for hash of cell_member_type"); - - std::size_t k = ((std::size_t)m.gid << (8*sizeof(cell_gid_type))) + m.index; - return std::hash<std::size_t>{}(k); + if (sizeof(std::size_t)>sizeof(cell_gid_type)) { + constexpr unsigned shift = 8*sizeof(cell_gid_type); + + std::size_t k = m.gid; + k <<= (shift/2); // dodge gcc shift warning when other branch taken + k <<= (shift/2); + k += m.index; + return std::hash<std::size_t>{}(k); + } + else { + constexpr std::size_t prime1 = 93481; + constexpr std::size_t prime2 = 54517; + + std::size_t k = prime1; + k = k*prime2 + m.gid; + k = k*prime2 + m.index; + return k; + } } }; } diff --git a/cmake/CompilerOptions.cmake b/cmake/CompilerOptions.cmake index f192db72..dac4325e 100644 --- a/cmake/CompilerOptions.cmake +++ b/cmake/CompilerOptions.cmake @@ -91,7 +91,7 @@ function(set_arch_target optvar arch) # Use -mcpu for all supported targets _except_ for x86, where it should be -march. - if(target_model MATCHES "x86" OR target_model MATCHES "amd64" OR target_model MATCHES "aarch64") + if(target_model MATCHES "x86|i[3456]86" OR target_model MATCHES "amd64" OR target_model MATCHES "aarch64") set(arch_opt "-march=${arch}") else() set(arch_opt "-mcpu=${arch}") diff --git a/test/unit/test_algorithms.cpp b/test/unit/test_algorithms.cpp index 76655b79..e77e3f99 100644 --- a/test/unit/test_algorithms.cpp +++ b/test/unit/test_algorithms.cpp @@ -663,7 +663,7 @@ TEST(algorithms, binary_find) auto ita = binary_find(a, 1); auto found = ita!=std::end(a); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(a), ita), 0u); + EXPECT_EQ(std::distance(std::begin(a), ita), 0); if (found) { EXPECT_EQ(*ita, 1); } @@ -672,7 +672,7 @@ TEST(algorithms, binary_find) auto itv = binary_find(v, 1); found = itv!=std::end(v); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(v), itv), 0u); + EXPECT_EQ(std::distance(std::begin(v), itv), 0); if (found) { EXPECT_EQ(*itv, 1); } @@ -684,7 +684,7 @@ TEST(algorithms, binary_find) auto ita = binary_find(a, 15); auto found = ita!=std::end(a); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(a), ita), 2u); + EXPECT_EQ(std::distance(std::begin(a), ita), 2); if (found) { EXPECT_EQ(*ita, 15); } @@ -693,7 +693,7 @@ TEST(algorithms, binary_find) auto itv = binary_find(v, 15); found = itv!=std::end(v); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(v), itv), 2u); + EXPECT_EQ(std::distance(std::begin(v), itv), 2); if (found) { EXPECT_EQ(*itv, 15); } @@ -705,7 +705,7 @@ TEST(algorithms, binary_find) auto ita = binary_find(a, 10); auto found = ita!=std::end(a); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(a), ita), 1u); + EXPECT_EQ(std::distance(std::begin(a), ita), 1); if (found) { EXPECT_EQ(*ita, 10); } @@ -714,7 +714,7 @@ TEST(algorithms, binary_find) auto itv = binary_find(v, 10); found = itv!=std::end(v); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(v), itv), 1u); + EXPECT_EQ(std::distance(std::begin(v), itv), 1); if (found) { EXPECT_EQ(*itv, 10); } @@ -726,7 +726,7 @@ TEST(algorithms, binary_find) auto ita = binary_find(a, 10); auto found = ita!=std::end(a); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(a), ita), 1u); + EXPECT_EQ(std::distance(std::begin(a), ita), 1); if (found) { EXPECT_EQ(*ita, 10); } @@ -735,7 +735,7 @@ TEST(algorithms, binary_find) auto itv = binary_find(v, 10); found = itv!=std::end(v); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::begin(v), itv), 1u); + EXPECT_EQ(std::distance(std::begin(v), itv), 1); if (found) { EXPECT_EQ(*itv, 10); } @@ -750,7 +750,7 @@ TEST(algorithms, binary_find) auto itv = binary_find(vr, 10); auto found = itv!=std::end(vr); EXPECT_TRUE(found); - EXPECT_EQ(std::distance(std::cbegin(v), itv), 1u); + EXPECT_EQ(std::distance(std::cbegin(v), itv), 1); if (found) { EXPECT_EQ(*itv, 10); } diff --git a/test/unit/test_simd.cpp b/test/unit/test_simd.cpp index 36bc74b3..47c701c7 100644 --- a/test/unit/test_simd.cpp +++ b/test/unit/test_simd.cpp @@ -664,7 +664,7 @@ TYPED_TEST_P(simd_fp_value, fp_maths) { fp exprelr_u[N]; for (unsigned i = 0; i<N; ++i) { - exprelr_u[i] = u[i]+fp(1)==fp(1)? fp(1): u[i]/(std::exp(u[i])-fp(1)); + exprelr_u[i] = u[i]+fp(1)==fp(1)? fp(1): u[i]/(std::expm1(u[i])); } exprelr(simd(u)).copy_to(r); EXPECT_TRUE(testing::seq_almost_eq<fp>(exprelr_u, r)); -- GitLab