Merge pull request #631 from intel/lukevalenty/pseudo_pext_lookup_indirect_search_fix

Fix two bugs related to pseudo pext indirect lookup with search

Author: elbeno

include/lookup/lookup.hpp

@@ -10,6 +10,6 @@
 namespace lookup {
 [[nodiscard]] CONSTEVAL static auto make(compile_time auto input) {
     return strategies<linear_search_lookup<4>,
-                      pseudo_pext_lookup<true, 1>>::make(input);
+                      pseudo_pext_lookup<true, 2>>::make(input);
 }
 } // namespace lookup

include/lookup/pseudo_pext_lookup.hpp

@@ -122,7 +122,7 @@ constexpr auto count_duplicates(std::array<T, S> keys) -> std::size_t {
     return dups;
 }
 
-/// count the length of the longest run of identical values (n log n)
+/// count the length of the longest run of identical values (n)
 template <typename T, std::size_t S>
 constexpr auto count_longest_run(std::array<T, S> keys) -> std::size_t {
     std::sort(keys.begin(), keys.end());
@@ -138,8 +138,11 @@ constexpr auto count_longest_run(std::array<T, S> keys) -> std::size_t {
 
             if (curr_value == prev_value) {
                 current_run++;
-            } else {
-                longest_run = std::max(longest_run, current_run);
+            }
+
+            longest_run = std::max(longest_run, current_run);
+
+            if (curr_value != prev_value) {
                 current_run = 0;
             }
 
@@ -241,15 +244,15 @@ constexpr auto calc_pseudo_pext_mask(std::array<entry<T, V>, S> const &pairs,
     while (max_search_len > 1 && std::popcount(mask) > 4) {
         auto try_mask = remove_cheapest_bit(mask, keys);
         auto current_longest_run = count_longest_run(with_mask(try_mask, keys));
-        if (current_longest_run < max_search_len) {
+        if (current_longest_run <= max_search_len) {
             mask = try_mask;
             prev_longest_run = current_longest_run;
         } else {
             return std::make_tuple(mask, prev_longest_run);
         }
     }
 
-    return std::make_tuple(mask, std::size_t{});
+    return std::make_tuple(mask, prev_longest_run);
 }
 
 } // namespace detail

test/lookup/pseudo_pext_lookup.cpp

@@ -98,3 +98,39 @@ TEMPLATE_TEST_CASE("lookup with scoped enum entries", "[pseudo pext lookup]",
     CHECK(lookup[some_key_t::KAPPA] == 87);
     CHECK(lookup[some_key_t::GAMMA] == 4);
 }
+
+TEST_CASE("pbt regression 0", "[pseudo pext lookup]") {
+    constexpr auto lookup = lookup::pseudo_pext_lookup<true, 2>::make(
+        CX_VALUE(lookup::input<std::uint16_t, std::uint16_t, 5>{
+            0, std::array<lookup::entry<std::uint16_t, std::uint16_t>, 5>{
+                   lookup::entry<std::uint16_t, std::uint16_t>{1, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{3, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{11, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{16, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{0, 1}}}));
+
+    CHECK(lookup[1] == 0);
+    CHECK(lookup[3] == 0);
+    CHECK(lookup[11] == 0);
+    CHECK(lookup[16] == 0);
+    CHECK(lookup[0] == 1);
+}
+
+TEST_CASE("pbt regression 1", "[pseudo pext lookup]") {
+    constexpr auto lookup = lookup::pseudo_pext_lookup<true, 2>::make(
+        CX_VALUE(lookup::input<std::uint16_t, std::uint16_t, 6>{
+            0, std::array<lookup::entry<std::uint16_t, std::uint16_t>, 6>{
+                   lookup::entry<std::uint16_t, std::uint16_t>{1, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{3, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{4, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{15, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{30, 0},
+                   lookup::entry<std::uint16_t, std::uint16_t>{31, 1}}}));
+
+    CHECK(lookup[1] == 0);
+    CHECK(lookup[3] == 0);
+    CHECK(lookup[4] == 0);
+    CHECK(lookup[15] == 0);
+    CHECK(lookup[30] == 0);
+    CHECK(lookup[31] == 1);
+}