Skip to content

Commit

Permalink
Fixed faulty bit field reduction in short pointers.
Browse files Browse the repository at this point in the history
  • Loading branch information
ujjwal-shekhar committed Oct 9, 2024
1 parent d1eb7e0 commit 91d692d
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 34 deletions.
4 changes: 2 additions & 2 deletions hhds/tests/add_append_bench/chip_typical_long_tree_bench.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ BENCHMARK(test_chip_typical_long_tree_4_hhds);
BENCHMARK(test_chip_typical_long_tree_4_lh);
BENCHMARK(test_chip_typical_long_tree_5_hhds);
BENCHMARK(test_chip_typical_long_tree_5_lh);
BENCHMARK(test_chip_typical_long_tree_6_hhds)->Iterations(3);
BENCHMARK(test_chip_typical_long_tree_6_lh)->Iterations(3);
// BENCHMARK(test_chip_typical_long_tree_6_hhds)->Iterations(3);
// BENCHMARK(test_chip_typical_long_tree_6_lh)->Iterations(3);
// BENCHMARK(test_chip_typical_long_tree_7_hhds)->Iterations(4);
// BENCHMARK(test_chip_typical_long_tree_7_lh)->Iterations(4);
// BENCHMARK(test_chip_typical_long_tree_8_hhds);
Expand Down
7 changes: 1 addition & 6 deletions hhds/tests/chip_typical_correctness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ void test_chip_tree() {
std::vector<lh::Tree_index> lh_current_level{lh::Tree_index(0, 0)};

int id = 1;
for (int depth = 0; depth < 4; ++depth) {
for (int depth = 0; depth < 8; ++depth) {
std::vector<hhds::Tree_pos> hhds_next_level;
std::vector<lh::Tree_index> lh_next_level;
std::vector<std::vector<int>> level_data;
Expand All @@ -110,17 +110,12 @@ void test_chip_tree() {
int num_children = generate_random_int(generator, 1, 7);
std::vector<int> children_data;

std::cout << "ADDING CHILDREN: " << num_children << " to " << hhds_node << std::endl;

for (int i = 0; i < num_children; ++i) {
int data = id++;
auto added = hhds_tree.add_child(hhds_node, data);

hhds_next_level.push_back(added);
children_data.push_back(data);

hhds_tree.print_tree(1);
std::cout << "-------------------" << std::endl;
}
level_data.push_back(children_data);
}
Expand Down
2 changes: 1 addition & 1 deletion hhds/tests/chip_typical_long_correctness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void test_chip_tree() {
std::vector<lh::Tree_index> lh_current_level{lh::Tree_index(0, 0)};

int id = 1;
for (int depth = 0; depth < 7; ++depth) {
for (int depth = 0; depth < 6; ++depth) {
std::vector<hhds::Tree_pos> hhds_next_level;
std::vector<lh::Tree_index> lh_next_level;
std::vector<std::vector<int>> level_data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,12 @@ BENCHMARK(test_chip_typical_long_tree_4_hhds);
BENCHMARK(test_chip_typical_long_tree_4_lh);
BENCHMARK(test_chip_typical_long_tree_5_hhds);
BENCHMARK(test_chip_typical_long_tree_5_lh);
BENCHMARK(test_chip_typical_long_tree_6_hhds);
BENCHMARK(test_chip_typical_long_tree_6_lh);
// BENCHMARK(test_chip_typical_long_tree_6_hhds);
// BENCHMARK(test_chip_typical_long_tree_6_lh);
// BENCHMARK(test_chip_typical_long_tree_7_hhds);
// BENCHMARK(test_chip_typical_long_tree_7_lh);
// BENCHMARK(test_chip_typical_long_tree_8_hhds);
// BENCHMARK(test_chip_typical_long_tree_8_lh);

// Run the benchmarks
BENCHMARK_MAIN();
33 changes: 10 additions & 23 deletions hhds/tree.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,16 @@ class __attribute__((packed, aligned(64))) Tree_pointers {
unsigned short num_occupied : CHUNK_SHIFT;

// Short (delta) child pointers
__int128 first_child_s : SHORT_DELTA;
__int128 last_child_s : SHORT_DELTA;
__int128 first_child_s;
__int128 last_child_s;

/* BUGGY START*/
// Helper functions to get and set first child pointers by index
inline Short_delta _get_first_child_s(short index) const {
return (first_child_s >> (index * SHORT_DELTA)) & ((1 << SHORT_DELTA) - 1);
}

inline void _set_first_child_s(short index, Short_delta value) {
first_child_s &= ~((__int128)((1 << SHORT_DELTA) - 1) << (index * SHORT_DELTA));
first_child_s &= ~((__int128)((static_cast<__int128>(1) << SHORT_DELTA) - 1) << (index * SHORT_DELTA));
first_child_s |= ((__int128)value << (index * SHORT_DELTA));
}

Expand All @@ -97,24 +96,9 @@ class __attribute__((packed, aligned(64))) Tree_pointers {
}

inline void _set_last_child_s(short index, Short_delta value) {
__int128 mask = (static_cast<__int128>(1) << SHORT_DELTA) - static_cast<__int128>(1);

// This is what the mask looks like before
// for (int i = 0; i < 128; i++) std::cout << !!(mask & (static_cast<__int128>(1) << i));
// std::cout << std::endl;

// We shift the mask to the left by the index * SHORT_DELTA (a window of 1s appearing SHORT_DELTA times, starting at index)
mask = mask << static_cast<__int128>(index * SHORT_DELTA);

// This is what the mask looks like after
// for (int i = 0; i < 128; i++) std::cout << !!(mask & (static_cast<__int128>(1) << i));
// std::cout << std::endl;
// BUG / UB: The mask does not change at all!!

last_child_s &= ~mask;
last_child_s |= (static_cast<__int128>(value) << (index * SHORT_DELTA));
last_child_s &= ~((__int128)((static_cast<__int128>(1) << SHORT_DELTA) - 1) << (index * SHORT_DELTA));
last_child_s |= ((__int128)value << (index * SHORT_DELTA));
}
/* BUGGY END*/

// :private

Expand Down Expand Up @@ -169,7 +153,9 @@ class __attribute__((packed, aligned(64))) Tree_pointers {
}

// Setter for num_occ
void set_num_occupied(unsigned short num) { num_occupied = num; }
void set_num_occupied(unsigned short num) {
num_occupied = num;
}

// Operators
constexpr bool operator==(const Tree_pointers& other) const {
Expand Down Expand Up @@ -1179,6 +1165,7 @@ Tree_pos tree<X>::insert_next_sibling(const Tree_pos& sibling_id, const X& data)
*/
template <typename X>
Tree_pos tree<X>::add_root(const X& data) {
I(pointers_stack.empty(), "add_root: Tree is not empty");
// if (!pointers_stack.empty()) {
// throw std::logic_error("add_root: Tree is not empty");
// }
Expand All @@ -1199,7 +1186,7 @@ Tree_pos tree<X>::add_root(const X& data) {
pointers_stack.emplace_back();

// Set num occupied to 1
pointers_stack[ROOT].set_num_occupied(0);
// pointers_stack[ROOT].set_num_occupied(0);

return (data_stack.size() - CHUNK_SIZE);
}
Expand Down

0 comments on commit 91d692d

Please sign in to comment.