Skip to content

Commit

Permalink
refactor: plain struct flavor entities (#3277)
Browse files Browse the repository at this point in the history
Remove array backing, propagating necessary changes. While we do use
structs as arrays, it's simpler conceptually to model this as just
another getter view instead of coupling strongly to an array storage
model. This gives more flexibility to how these are defined, and give
less gotchas via reference copying.

This requires more definition of `pointer_view()`. Note that the main
mechanism for ensuring `pointer_view()` is correct is checking that the
number of listed members is the one expected (e.g. equals
NUM_ALL_ENTITIES).

Review notes:
- A bunch of loops had to rethought as they previously did random
access. Random access is possible in a sense, but only after
constructing a pointer_view which, while cheap, is not free. As a result
there's been some refactoring
- zip_view has been used where possible, especially to get around the
awkwardness of first creating the `pointer_view` and then using it as a
structure to index into

---------

Co-authored-by: ludamad <[email protected]>
  • Loading branch information
ludamad and ludamad0 authored Nov 13, 2023
1 parent db0f3ab commit f109512
Show file tree
Hide file tree
Showing 21 changed files with 1,340 additions and 1,092 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ template <typename Curve> class ZeroMorphProver_ {

// Extract multilinear challenge u and claimed multilinear evaluations from Sumcheck output
std::span<FF> u_challenge = multilinear_challenge;
std::span<FF> claimed_evaluations = evaluations;
auto claimed_evaluations = evaluations.pointer_view();
size_t log_N = u_challenge.size();
size_t N = 1 << log_N;

Expand All @@ -340,14 +340,14 @@ template <typename Curve> class ZeroMorphProver_ {
size_t poly_idx = 0; // TODO(#391) zip
for (auto& f_poly : f_polynomials) {
f_batched.add_scaled(f_poly, rhos[poly_idx]);
batched_evaluation += rhos[poly_idx] * claimed_evaluations[poly_idx];
batched_evaluation += rhos[poly_idx] * (*claimed_evaluations[poly_idx]);
++poly_idx;
}

Polynomial g_batched(N); // batched to-be-shifted polynomials
for (auto& g_poly : g_polynomials) {
g_batched.add_scaled(g_poly, rhos[poly_idx]);
batched_evaluation += rhos[poly_idx] * claimed_evaluations[poly_idx];
batched_evaluation += rhos[poly_idx] * (*claimed_evaluations[poly_idx]);
++poly_idx;
};

Expand Down
12 changes: 12 additions & 0 deletions barretenberg/cpp/src/barretenberg/common/serialize.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,18 @@ template <typename B, typename T> inline void read(B& it, std::optional<T>& opt_
opt_value = T(value);
}

template <typename T>
concept HasPointerView = requires(T t) { t.pointer_view(); };

// Write out a struct that defines pointer_view()
template <typename B, HasPointerView T> inline void write(B& buf, T const& value)
{
using serialize::write;
for (auto* pointer : value.pointer_view()) {
write(buf, *pointer);
}
}

// Write std::optional<T>.
// Note: It takes up a different amount of space, depending on whether it's std::nullopt or populated with an actual
// value.
Expand Down
491 changes: 272 additions & 219 deletions barretenberg/cpp/src/barretenberg/flavor/ecc_vm.hpp

Large diffs are not rendered by default.

35 changes: 13 additions & 22 deletions barretenberg/cpp/src/barretenberg/flavor/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,20 @@

namespace proof_system::honk::flavor {

template <std::size_t ExpectedSize, typename T, std::size_t N> static auto _assert_array_size(std::array<T, N>&& array)
{
static_assert(N == ExpectedSize,
"Expected array size to match given size (first parameter) in DEFINE_POINTER_VIEW");
return array;
}

#define DEFINE_POINTER_VIEW(ExpectedSize, ...) \
[[nodiscard]] auto pointer_view() \
{ \
return _assert_array_size<ExpectedSize>(std::array{ __VA_ARGS__ }); \
std::array view{ __VA_ARGS__ }; \
static_assert(view.size() == ExpectedSize, \
"Expected array size to match given size (first parameter) in DEFINE_POINTER_VIEW"); \
return view; \
} \
[[nodiscard]] auto pointer_view() const \
{ \
return _assert_array_size<ExpectedSize>(std::array{ __VA_ARGS__ }); \
std::array view{ __VA_ARGS__ }; \
static_assert(view.size() == ExpectedSize, \
"Expected array size to match given size (first parameter) in DEFINE_POINTER_VIEW"); \
return view; \
}

/**
Expand All @@ -101,13 +100,7 @@ template <std::size_t ExpectedSize, typename T, std::size_t N> static auto _asse
*/
template <typename DataType, typename HandleType, size_t NUM_ENTITIES> class Entities_ {
public:
using ArrayType = std::array<DataType, NUM_ENTITIES>;
ArrayType _data;

virtual ~Entities_() = default;
// TODO(AD): remove these with the backing array
typename ArrayType::iterator begin() { return _data.begin(); };
typename ArrayType::iterator end() { return _data.end(); };

constexpr size_t size() { return NUM_ENTITIES; };
};
Expand Down Expand Up @@ -154,13 +147,11 @@ class ProvingKey_ : public PrecomputedPolynomials, public WitnessPolynomials {
using Polynomial = typename PrecomputedPolynomials::DataType;
using FF = typename Polynomial::FF;

typename PrecomputedPolynomials::ArrayType& _precomputed_polynomials = PrecomputedPolynomials::_data;
typename WitnessPolynomials::ArrayType& _witness_polynomials = WitnessPolynomials::_data;

bool contains_recursive_proof;
std::vector<uint32_t> recursive_proof_public_input_indices;
barretenberg::EvaluationDomain<FF> evaluation_domain;

auto precomputed_polynomials_pointer_view() { return PrecomputedPolynomials::pointer_view(); }
ProvingKey_() = default;
ProvingKey_(const size_t circuit_size, const size_t num_public_inputs)
{
Expand All @@ -169,12 +160,12 @@ class ProvingKey_ : public PrecomputedPolynomials, public WitnessPolynomials {
this->log_circuit_size = numeric::get_msb(circuit_size);
this->num_public_inputs = num_public_inputs;
// Allocate memory for precomputed polynomials
for (auto& poly : _precomputed_polynomials) {
poly = Polynomial(circuit_size);
for (auto* poly : PrecomputedPolynomials::pointer_view()) {
*poly = Polynomial(circuit_size);
}
// Allocate memory for witness polynomials
for (auto& poly : _witness_polynomials) {
poly = Polynomial(circuit_size);
for (auto* poly : WitnessPolynomials::pointer_view()) {
*poly = Polynomial(circuit_size);
}
};
};
Expand Down
5 changes: 2 additions & 3 deletions barretenberg/cpp/src/barretenberg/flavor/flavor.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,8 @@ TEST(Flavor, GetRow)
});
Flavor::ProverPolynomials prover_polynomials;
size_t poly_idx = 0;
for (auto& poly : prover_polynomials) {
poly = data[poly_idx];
poly_idx++;
for (auto [poly, entry] : zip_view(prover_polynomials.pointer_view(), data)) {
*poly = entry;
}
auto row0 = prover_polynomials.get_row(0);
auto row1 = prover_polynomials.get_row(1);
Expand Down
Loading

0 comments on commit f109512

Please sign in to comment.