Skip to content

Commit 50806db

Browse files
AntoinePrvAntoinePrv
committed
Address review
1 parent afe91c8 commit 50806db

5 files changed

Lines changed: 53 additions & 22 deletions

File tree

‎include/xsimd/arch/xsimd_avx512f.hpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "../types/xsimd_avx512f_register.hpp"
1616
#include "../types/xsimd_batch_constant.hpp"
17+
#include "../utils/bits.hpp"
1718

1819
#include <complex>
1920
#include <limits>
@@ -2255,11 +2256,8 @@ namespace xsimd
22552256
XSIMD_INLINE batch_bool<T, A> set(batch_bool<T, A> const&, requires_arch<avx512f>, Values... values) noexcept
22562257
{
22572258
static_assert(sizeof...(Values) == batch_bool<T, A>::size, "consistent init");
2258-
using register_type = typename batch_bool<T, A>::register_type;
2259-
register_type r = 0;
2260-
unsigned shift = 0;
2261-
((r |= register_type(values ? 1 : 0) << (shift++)), ...);
2262-
return r;
2259+
using reg_t = typename batch_bool<T, A>::register_type;
2260+
return ::xsimd::utils::make_bit_mask_from_bools<reg_t>(values...);
22632261
}
22642262

22652263
// shuffle

‎include/xsimd/arch/xsimd_avx512vl_128.hpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "../types/xsimd_avx512vl_register.hpp"
1717
#include "../types/xsimd_batch_constant.hpp"
18+
#include "../utils/bits.hpp"
1819

1920
#include <type_traits>
2021

@@ -173,11 +174,8 @@ namespace xsimd
173174
XSIMD_INLINE batch_bool<T, A> set(batch_bool<T, A> const&, requires_arch<avx512vl_128>, Values... values) noexcept
174175
{
175176
static_assert(sizeof...(Values) == batch_bool<T, A>::size, "consistent init");
176-
using register_type = typename batch_bool<T, A>::register_type;
177-
register_type r = 0;
178-
unsigned shift = 0;
179-
((r |= register_type(values ? 1 : 0) << (shift++)), ...);
180-
return r;
177+
using reg_t = typename batch_bool<T, A>::register_type;
178+
return ::xsimd::utils::make_bit_mask_from_bools<reg_t>(values...);
181179
}
182180

183181
// store

‎include/xsimd/arch/xsimd_avx512vl_256.hpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "../types/xsimd_avx512vl_register.hpp"
1717
#include "../types/xsimd_batch_constant.hpp"
18+
#include "../utils/bits.hpp"
1819

1920
#include <type_traits>
2021

@@ -173,11 +174,8 @@ namespace xsimd
173174
XSIMD_INLINE batch_bool<T, A> set(batch_bool<T, A> const&, requires_arch<avx512vl_256>, Values... values) noexcept
174175
{
175176
static_assert(sizeof...(Values) == batch_bool<T, A>::size, "consistent init");
176-
using register_type = typename batch_bool<T, A>::register_type;
177-
register_type r = 0;
178-
unsigned shift = 0;
179-
((r |= register_type(values ? 1 : 0) << (shift++)), ...);
180-
return r;
177+
using reg_t = typename batch_bool<T, A>::register_type;
178+
return ::xsimd::utils::make_bit_mask_from_bools<reg_t>(values...);
181179
}
182180

183181
// store

‎include/xsimd/utils/bits.hpp‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,40 @@ namespace xsimd
1919
{
2020
namespace utils
2121
{
22-
template <typename I, typename... Args>
23-
constexpr I make_bit_mask(I bit, Args... bits)
22+
template <typename... Args>
23+
constexpr auto make_bit_mask(Args... bits)
24+
{
25+
using out_type = std::common_type_t<unsigned short, std::make_unsigned_t<Args>...>;
26+
[[maybe_unused]] constexpr auto bit_count = static_cast<out_type>(8 * sizeof(out_type));
27+
assert((((static_cast<out_type>(bits) < bit_count) && ...)));
28+
return static_cast<out_type>((0u | ... | (1u << bits)));
29+
}
30+
31+
/**
32+
* Return a mask whose bit `i` holds the truth value of the `i`th argument.
33+
*/
34+
template <typename I, typename... Bools>
35+
constexpr I make_bit_mask_from_bools(Bools... bools) noexcept
2436
{
2537
static_assert(std::is_unsigned_v<I>, "Bit operations must be done on unsigned integers");
26-
[[maybe_unused]] constexpr I bit_count = static_cast<I>(8 * sizeof(I));
27-
assert(((bit < bit_count) && ... && (static_cast<I>(bits) < bit_count)));
28-
return static_cast<I>(((I { 1 } << bit) | ... | (I { 1 } << static_cast<I>(bits))));
38+
static_assert(sizeof...(Bools) <= 8 * sizeof(I), "Not enough bits to hold all the values");
39+
// GCC's -Wsequence-point does not model fold sequencing and rejects `shift++` inside
40+
// the fold operand, hence the comma to separate read from increment.
41+
I mask = 0;
42+
unsigned shift = 0;
43+
((mask |= static_cast<I>(static_cast<I>(bools ? 1 : 0) << shift), ++shift), ...);
44+
return mask;
2945
}
3046

31-
template <int... Bits, typename I>
47+
template <auto... Bits, typename I>
3248
constexpr bool all_bits_set(I value)
3349
{
3450
static_assert(std::is_unsigned_v<I>, "Bit operations must be done on unsigned integers");
3551
constexpr I mask = make_bit_mask<I>(static_cast<I>(Bits)...);
3652
return (value & mask) == mask;
3753
}
3854

39-
template <int Bit, typename I>
55+
template <auto Bit, typename I>
4056
constexpr I set_bit(I value)
4157
{
4258
static_assert(std::is_unsigned_v<I>, "Bit operations must be done on unsigned integers");

‎test/test_utils_bits.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
TEST_CASE("[utils::make_bit_mask] single bit")
1919
{
20+
CHECK_EQ(xsimd::utils::make_bit_mask(), 0x0);
21+
CHECK_EQ(xsimd::utils::make_bit_mask(1), 0b0010);
2022
CHECK_EQ(xsimd::utils::make_bit_mask<std::uint8_t>(0), 0x01);
2123
CHECK_EQ(xsimd::utils::make_bit_mask<std::uint8_t>(7), 0x80);
2224
CHECK_EQ(xsimd::utils::make_bit_mask<std::uint32_t>(0), 0x01u);
@@ -29,6 +31,25 @@ TEST_CASE("[utils::make_bit_mask] multiple bits")
2931
CHECK_EQ(xsimd::utils::make_bit_mask<std::uint8_t>(0, 2, 4), 0b00010101);
3032
}
3133

34+
TEST_CASE("[utils::make_bit_mask_from_bools]")
35+
{
36+
CHECK_EQ(xsimd::utils::make_bit_mask_from_bools<std::uint8_t>(), 0x00);
37+
CHECK_EQ(xsimd::utils::make_bit_mask_from_bools<std::uint8_t>(true), 0x01);
38+
CHECK_EQ(xsimd::utils::make_bit_mask_from_bools<std::uint8_t>(false), 0x00);
39+
// First argument goes in the lowest bit
40+
CHECK_EQ(xsimd::utils::make_bit_mask_from_bools<std::uint8_t>(true, false, false), 0b001);
41+
CHECK_EQ(xsimd::utils::make_bit_mask_from_bools<std::uint8_t>(false, false, true), 0b100);
42+
// Full width
43+
CHECK_EQ(
44+
xsimd::utils::make_bit_mask_from_bools<std::uint8_t>(true, true, true, true, true, true, true, true),
45+
0xFF);
46+
CHECK_EQ(
47+
xsimd::utils::make_bit_mask_from_bools<std::uint16_t>(
48+
true, false, true, false, true, false, true, false,
49+
true, false, true, false, true, false, true, false),
50+
0x5555);
51+
}
52+
3253
TEST_CASE("[utils::all_bits_set] basic")
3354
{
3455
CHECK(xsimd::utils::all_bits_set<0>(0x01u));

0 commit comments

Comments
 (0)