Skip to content

Commit

Permalink
Revert "refactor(bb): allow dynamic plookup tables" (#4672)
Browse files Browse the repository at this point in the history 
Reverts #4667

Turned out to be more involved than thought + not planned imminently
  • Loading branch information
@ludamad
ludamad authored Feb 19, 2024
1 parent 2dd53fd commit ba6048d
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 62 deletions.
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,12 +633,12 @@ plookup::BasicTable& UltraCircuitBuilder_<Arithmetization>::get_table(const ploo

template <typename Arithmetization>
plookup::ReadData<uint32_t> UltraCircuitBuilder_<Arithmetization>::create_gates_from_plookup_accumulators(
const plookup::MultiTableIdOrPtr& id,
const plookup::MultiTableId& id,
const plookup::ReadData<FF>& read_values,
const uint32_t key_a_index,
std::optional<uint32_t> key_b_index)
{
const auto& multi_table = plookup::get_table(id);
const auto& multi_table = plookup::create_table(id);
const size_t num_lookups = read_values[plookup::ColumnIdx::C1].size();
plookup::ReadData<uint32_t> read_data;
for (size_t i = 0; i < num_lookups; ++i) {
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization:
plookup::MultiTable& create_table(const plookup::MultiTableId id);

plookup::ReadData<uint32_t> create_gates_from_plookup_accumulators(
const plookup::MultiTableIdOrPtr& id,
const plookup::MultiTableId& id,
const plookup::ReadData<FF>& read_values,
const uint32_t key_a_index,
std::optional<uint32_t> key_b_index = std::nullopt);
Expand Down
12 changes: 2 additions & 10 deletions barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,13 @@ const MultiTable& create_table(const MultiTableId id)
return MULTI_TABLES[id];
}

const MultiTable& get_table(const MultiTableIdOrPtr& id)
{
if (id.ptr == nullptr) {
return create_table(id.id);
}
return *id.ptr;
}

ReadData<bb::fr> get_lookup_accumulators(const MultiTableIdOrPtr& id,
ReadData<bb::fr> get_lookup_accumulators(const MultiTableId id,
const fr& key_a,
const fr& key_b,
const bool is_2_to_1_lookup)
{
// return multi-table, populating global array of all multi-tables if need be
const auto& multi_table = get_table(id);
const auto& multi_table = create_table(id);
const size_t num_lookups = multi_table.lookup_ids.size();

ReadData<bb::fr> lookup;
Expand Down
3 changes: 1 addition & 2 deletions barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
namespace bb::plookup {

const MultiTable& create_table(MultiTableId id);
const MultiTable& get_table(const MultiTableIdOrPtr& id);

ReadData<bb::fr> get_lookup_accumulators(const MultiTableIdOrPtr& id,
ReadData<bb::fr> get_lookup_accumulators(MultiTableId id,
const bb::fr& key_a,
const bb::fr& key_b = 0,
bool is_2_to_1_lookup = false);
Expand Down
88 changes: 66 additions & 22 deletions barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,17 @@ struct MultiTable {
std::vector<bb::fr> column_1_step_sizes;
std::vector<bb::fr> column_2_step_sizes;
std::vector<bb::fr> column_3_step_sizes;
using table_out = std::array<bb::fr, 2>;
using table_in = std::array<uint64_t, 2>;
typedef std::array<bb::fr, 2> table_out;
typedef std::array<uint64_t, 2> table_in;
std::vector<table_out (*)(table_in)> get_table_values;

private:
void init_step_sizes()
{
const size_t num_lookups = column_1_coefficients.size();
column_1_step_sizes.emplace_back(1);
column_2_step_sizes.emplace_back(1);
column_3_step_sizes.emplace_back(1);
column_1_step_sizes.emplace_back(bb::fr(1));
column_2_step_sizes.emplace_back(bb::fr(1));
column_3_step_sizes.emplace_back(bb::fr(1));

std::vector<bb::fr> coefficient_inverses(column_1_coefficients.begin(), column_1_coefficients.end());
std::copy(column_2_coefficients.begin(), column_2_coefficients.end(), std::back_inserter(coefficient_inverses));
Expand Down Expand Up @@ -184,30 +184,74 @@ struct MultiTable {
init_step_sizes();
}

MultiTable() = default;
MultiTable(){};
MultiTable(const MultiTable& other) = default;
MultiTable(MultiTable&& other) = default;

MultiTable& operator=(const MultiTable& other) = default;
MultiTable& operator=(MultiTable&& other) = default;
};

// Represents either a predefined table from our enum list of supported lookup tables, or a dynamic lookup table defined
// by ACIR
struct MultiTableIdOrPtr {
// Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused.
MultiTableId id;
// Used if we are using a lookup table from a lookup table defined by e.g. ACIR, otherwise set to nullptr.
MultiTable* ptr;
MultiTableIdOrPtr(MultiTable* ptr)
: id(NUM_MULTI_TABLES)
, ptr(ptr)
{}
MultiTableIdOrPtr(MultiTableId id)
: id(id)
, ptr(nullptr)
{}
};
// struct PlookupLargeKeyTable {
// struct KeyEntry {
// uint256_t key;
// std::array<bb::fr, 2> value{ bb::fr(0), bb::fr(0) };
// bool operator<(const KeyEntry& other) const { return key < other.key; }

// std::array<bb::fr, 3> to_sorted_list_components(const bool use_two_keys) const
// {
// return {
// key[0],
// value[0],
// value[1],
// };
// }
// };

// BasicTableId id;
// size_t table_index;
// size_t size;
// bool use_twin_keys;

// bb::fr column_1_step_size = bb::fr(0);
// bb::fr column_2_step_size = bb::fr(0);
// bb::fr column_3_step_size = bb::fr(0);
// std::vector<bb::fr> column_1;
// std::vector<bb::fr> column_3;
// std::vector<bb::fr> column_2;
// std::vector<KeyEntry> lookup_gates;

// std::array<bb::fr, 2> (*get_values_from_key)(const std::array<uint64_t, 2>);
// };

// struct PlookupFatKeyTable {
// struct KeyEntry {
// bb::fr key;
// std::array<bb::fr, 2> values{ 0, 0 };
// bool operator<(const KeyEntry& other) const
// {
// return (key.from_montgomery_form() < other.key.from_montgomery_form());
// }

// std::array<bb::fr, 3> to_sorted_list_components() const { return { key, values[0], values[0] }; }
// }

// BasicTableId id;
// size_t table_index;
// size_t size;
// bool use_twin_keys;

// bb::fr column_1_step_size = bb::fr(0);
// bb::fr column_2_step_size = bb::fr(0);
// bb::fr column_3_step_size = bb::fr(0);
// std::vector<bb::fr> column_1;
// std::vector<bb::fr> column_3;
// std::vector<bb::fr> column_2;
// std::vector<KeyEntry> lookup_gates;

// std::array<bb::fr, 2> (*get_values_from_key)(const std::array<uint64_t, 2>);

// }

/**
* @brief The structure contains the most basic table serving one function (for, example an xor table)
Expand Down
11 changes: 5 additions & 6 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ using plookup::MultiTableId;
using namespace bb;

template <typename Builder>
plookup::ReadData<field_t<Builder>> plookup_read<Builder>::get_lookup_accumulators(const plookup::MultiTableIdOrPtr& id,
plookup::ReadData<field_t<Builder>> plookup_read<Builder>::get_lookup_accumulators(const MultiTableId id,
const field_t<Builder>& key_a_in,
const field_t<Builder>& key_b_in,
const bool is_2_to_1_lookup)
Expand Down Expand Up @@ -64,16 +64,16 @@ plookup::ReadData<field_t<Builder>> plookup_read<Builder>::get_lookup_accumulato
}

template <typename Builder>
std::pair<field_t<Builder>, field_t<Builder>> plookup_read<Builder>::read_pair_from_table(
const plookup::MultiTableIdOrPtr& id, const field_t<Builder>& key)
std::pair<field_t<Builder>, field_t<Builder>> plookup_read<Builder>::read_pair_from_table(const MultiTableId id,
const field_t<Builder>& key)
{
const auto lookup = get_lookup_accumulators(id, key);

return { lookup[ColumnIdx::C2][0], lookup[ColumnIdx::C3][0] };
}

template <typename Builder>
field_t<Builder> plookup_read<Builder>::read_from_2_to_1_table(const plookup::MultiTableIdOrPtr& id,
field_t<Builder> plookup_read<Builder>::read_from_2_to_1_table(const MultiTableId id,
const field_t<Builder>& key_a,
const field_t<Builder>& key_b)
{
Expand All @@ -83,8 +83,7 @@ field_t<Builder> plookup_read<Builder>::read_from_2_to_1_table(const plookup::Mu
}

template <typename Builder>
field_t<Builder> plookup_read<Builder>::read_from_1_to_2_table(const plookup::MultiTableIdOrPtr& id,
const field_t<Builder>& key_a)
field_t<Builder> plookup_read<Builder>::read_from_1_to_2_table(const MultiTableId id, const field_t<Builder>& key_a)
{
const auto lookup = get_lookup_accumulators(id, key_a);

Expand Down
9 changes: 4 additions & 5 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@ template <typename Builder> class plookup_read {
typedef field_t<Builder> field_pt;

public:
static std::pair<field_pt, field_pt> read_pair_from_table(const plookup::MultiTableIdOrPtr& id,
const field_pt& key);
static std::pair<field_pt, field_pt> read_pair_from_table(const plookup::MultiTableId id, const field_pt& key);

static field_pt read_from_2_to_1_table(const plookup::MultiTableIdOrPtr& id,
static field_pt read_from_2_to_1_table(const plookup::MultiTableId id,
const field_pt& key_a,
const field_pt& key_b);
static field_pt read_from_1_to_2_table(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a);
static field_pt read_from_1_to_2_table(const plookup::MultiTableId id, const field_pt& key_a);

static plookup::ReadData<field_pt> get_lookup_accumulators(const plookup::MultiTableIdOrPtr& id,
static plookup::ReadData<field_pt> get_lookup_accumulators(const plookup::MultiTableId id,
const field_pt& key_a,
const field_pt& key_b = 0,
const bool is_2_to_1_lookup = false);
Expand Down
16 changes: 2 additions & 14 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ TEST(stdlib_plookup, blake2s_xor)
EXPECT_EQ(result, true);
}

static void test_uint32_and(const MultiTableIdOrPtr& id)
TEST(stdlib_plookup, uint32_and)
{
Builder builder = Builder();

Expand All @@ -480,7 +480,7 @@ static void test_uint32_and(const MultiTableIdOrPtr& id)
field_ct left = witness_ct(&builder, bb::fr(left_value));
field_ct right = witness_ct(&builder, bb::fr(right_value));

const auto lookup = plookup_read::get_lookup_accumulators(id, left, right, true);
const auto lookup = plookup_read::get_lookup_accumulators(MultiTableId::UINT32_AND, left, right, true);
const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups);
const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups);
std::vector<uint256_t> out_expected(num_lookups);
Expand Down Expand Up @@ -510,18 +510,6 @@ static void test_uint32_and(const MultiTableIdOrPtr& id)
EXPECT_EQ(result, true);
}

// Tests the dynamic multitable interface used by ACIR (the Noir interface to bb)
TEST(stdlib_plookup, dynamic_uint32_and)
{
MultiTable and_table = bb::plookup::uint_tables::get_uint32_and_table();
test_uint32_and(&and_table);
}

TEST(stdlib_plookup, uint32_and)
{
test_uint32_and(MultiTableId::UINT32_AND);
}

TEST(stdlib_plookup, secp256k1_generator)
{
using curve = stdlib::secp256k1<Builder>;
Expand Down

0 comments on commit ba6048d

Please sign in to comment.