Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(avm): correctly build spike vm #7726

Merged
merged 1 commit into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions barretenberg/cpp/src/barretenberg/flavor/flavor_macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@
#include "barretenberg/common/std_array.hpp"
#include "barretenberg/common/std_string.hpp"
#include "barretenberg/common/std_vector.hpp"

#include <array>
#include <iostream>
#include <sstream>
#include <type_traits>

namespace bb::detail {

template <typename... Args> constexpr std::size_t _va_count(Args&&... /*unused*/)
{
return sizeof...(Args);
Expand All @@ -35,19 +38,21 @@ template <typename T, typename... BaseClass> auto _concatenate_base_class_get_la
{
return concatenate(static_cast<const BaseClass&>(arg).get_labels()...);
}

} // namespace bb::detail

// Needed to force expansion of __VA_ARGS__ before converting to string.
#define VARARGS_TO_STRING(...) #__VA_ARGS__

// We use std::remove_reference to support a flavor that has references as members. This is an AVM use case.
#define DEFINE_REF_VIEW(...) \
[[nodiscard]] auto get_all() \
{ \
return RefArray{ __VA_ARGS__ }; \
return RefArray<std::remove_reference_t<DataType>, _members_size>{ __VA_ARGS__ }; \
} \
[[nodiscard]] auto get_all() const \
{ \
return RefArray{ __VA_ARGS__ }; \
return RefArray<const std::remove_reference_t<DataType>, _members_size>{ __VA_ARGS__ }; \
}

/**
Expand All @@ -58,17 +63,18 @@ template <typename T, typename... BaseClass> auto _concatenate_base_class_get_la
* @tparam NUM_ENTITIES The size of the underlying array.
*/
#define DEFINE_FLAVOR_MEMBERS(DataType, ...) \
DataType __VA_ARGS__; \
__VA_OPT__(DataType __VA_ARGS__;) \
static constexpr size_t _members_size = std::tuple_size<decltype(std::make_tuple(__VA_ARGS__))>::value; \
DEFINE_REF_VIEW(__VA_ARGS__) \
const std::vector<std::string>& get_labels() const \
static const std::vector<std::string>& get_labels() \
{ \
static const std::vector<std::string> labels = \
bb::detail::split_and_trim(VARARGS_TO_STRING(__VA_ARGS__), ','); \
return labels; \
} \
constexpr std::size_t size() const \
static constexpr std::size_t size() \
{ \
return bb::detail::_va_count(__VA_ARGS__); \
return _members_size; \
}

#define DEFINE_COMPOUND_GET_ALL(...) \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

namespace bb {

AvmFlavor::AllConstRefValues::AllConstRefValues(const RefArray<FF const, AvmFlavor::NUM_ALL_ENTITIES>& il)
AvmFlavor::AllConstRefValues::AllConstRefValues(
const RefArray<AvmFlavor::AllConstRefValues::BaseDataType, AvmFlavor::NUM_ALL_ENTITIES>& il)
: main_clk(il[0])
, main_sel_first(il[1])
, kernel_kernel_inputs(il[2])
Expand Down
11 changes: 7 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,11 @@ class AvmFlavor {
};

template <typename DataType, typename PrecomputedAndWitnessEntitiesSuperset>
static auto get_to_be_shifted(PrecomputedAndWitnessEntitiesSuperset& entities)
static auto get_to_be_shifted([[maybe_unused]] PrecomputedAndWitnessEntitiesSuperset& entities)
{
return RefArray{ TO_BE_SHIFTED(entities) };
return RefArray<DataType, std::tuple_size<decltype(std::make_tuple(TO_BE_SHIFTED(entities)))>::value>{
TO_BE_SHIFTED(entities)
};
}

template <typename DataType>
Expand Down Expand Up @@ -302,11 +304,12 @@ class AvmFlavor {

class AllConstRefValues {
public:
using DataType = const FF&;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh... I can't do this if I want to avoid copying the fields.

I had to do it because, while I DO want const FF& members, I want get_all etc to return RefArray<const FF> and not RefArray<const FF&> which is not possible.

I'll have to see how to solve this. Maybe removing the & when passing the type to RefArray (which is a bit of a hack).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, with std::remove_reference_t.

using BaseDataType = const FF;
using DataType = BaseDataType&;

DEFINE_FLAVOR_MEMBERS(DataType, ALL_ENTITIES)

AllConstRefValues(const RefArray<FF const, NUM_ALL_ENTITIES>& il);
AllConstRefValues(const RefArray<BaseDataType, NUM_ALL_ENTITIES>& il);
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ using FF = AvmFlavor::FF;
* @brief This function verifies an Avm Honk proof for given program settings.
*
*/
bool AvmVerifier::verify_proof(const HonkProof& proof, const std::vector<std::vector<FF>>& public_inputs)
bool AvmVerifier::verify_proof(const HonkProof& proof,
[[maybe_unused]] const std::vector<std::vector<FF>>& public_inputs)
{
using Flavor = AvmFlavor;
using FF = Flavor::FF;
Expand Down
2 changes: 1 addition & 1 deletion bb-pilcom/bb-pil-backend/templates/flavor.cpp.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace bb {

{{name}}Flavor::AllConstRefValues::AllConstRefValues(const RefArray<FF const, AvmFlavor::NUM_ALL_ENTITIES>& il) :
{{name}}Flavor::AllConstRefValues::AllConstRefValues(const RefArray<{{name}}Flavor::AllConstRefValues::BaseDataType, {{name}}Flavor::NUM_ALL_ENTITIES>& il) :
{{#each (join fixed witness_without_inverses lookups shifted) as |item|}}
{{item}}(il[{{@index}}]){{#unless @last}},{{/unless}}
{{/each}}
Expand Down
13 changes: 7 additions & 6 deletions bb-pilcom/bb-pil-backend/templates/flavor.hpp.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ template <typename... input_t> using tuple_cat_t = decltype(std::tuple_cat(std::
#define DERIVED_WITNESS_ENTITIES {{#each lookups as |item|}}{{#if @index}}, {{/if}}{{item}}{{/each}}
#define SHIFTED_ENTITIES {{#each shifted as |item|}}{{#if @index}}, {{/if}}{{item}}{{/each}}
#define TO_BE_SHIFTED(e) {{#each to_be_shifted as |item|}}{{#if @index}}, {{/if}}e.{{item}}{{/each}}
#define ALL_ENTITIES PRECOMPUTED_ENTITIES, WIRE_ENTITIES, DERIVED_WITNESS_ENTITIES, SHIFTED_ENTITIES
#define ALL_ENTITIES {{#if (len fixed)}}PRECOMPUTED_ENTITIES{{/if}}{{#if (len witness_without_inverses)}}, WIRE_ENTITIES{{/if}}{{#if (len lookups)}}, DERIVED_WITNESS_ENTITIES{{/if}}{{#if (len shifted)}}, SHIFTED_ENTITIES{{/if}}
// clang-format on

namespace bb {
Expand Down Expand Up @@ -123,8 +123,8 @@ class {{name}}Flavor {
};

template <typename DataType, typename PrecomputedAndWitnessEntitiesSuperset>
static auto get_to_be_shifted(PrecomputedAndWitnessEntitiesSuperset& entities) {
return RefArray{ TO_BE_SHIFTED(entities) };
static auto get_to_be_shifted([[maybe_unused]] PrecomputedAndWitnessEntitiesSuperset& entities) {
return RefArray<DataType, std::tuple_size<decltype(std::make_tuple(TO_BE_SHIFTED(entities)))>::value>{ TO_BE_SHIFTED(entities) };
}

template <typename DataType>
Expand Down Expand Up @@ -154,7 +154,7 @@ class {{name}}Flavor {
class ProvingKey : public ProvingKeyAvm_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey> {
public:
// Expose constructors on the base class
using Base = ProvingKey{{name}}_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base = ProvingKeyAvm_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
using Base::Base;

auto get_to_be_shifted() {
Expand All @@ -174,7 +174,8 @@ class {{name}}Flavor {
{{!-- Used by get_row, logderivs, etc --}}
class AllConstRefValues {
public:
using DataType = const FF&;
using BaseDataType = const FF;
using DataType = BaseDataType&;

{{!--
We define the flavor members here again to avoid having to make this class inherit from AllEntities.
Expand All @@ -183,7 +184,7 @@ class {{name}}Flavor {
--}}
DEFINE_FLAVOR_MEMBERS(DataType, ALL_ENTITIES)

AllConstRefValues(const RefArray<FF const, NUM_ALL_ENTITIES>& il);
AllConstRefValues(const RefArray<BaseDataType, NUM_ALL_ENTITIES>& il);
};

/**
Expand Down
2 changes: 1 addition & 1 deletion bb-pilcom/bb-pil-backend/templates/prover.cpp.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void {{name}}Prover::execute_wire_commitments_round()
}
}

void AvmProver::execute_log_derivative_inverse_round()
void {{name}}Prover::execute_log_derivative_inverse_round()
{
auto [beta, gamm] = transcript->template get_challenges<FF>("beta", "gamma");
relation_parameters.beta = beta;
Expand Down
2 changes: 1 addition & 1 deletion bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ using FF = {{name}}Flavor::FF;
* @brief This function verifies an {{name}} Honk proof for given program settings.
*
*/
bool {{name}}Verifier::verify_proof(const HonkProof& proof, const std::vector<std::vector<FF>>& public_inputs)
bool {{name}}Verifier::verify_proof(const HonkProof& proof, [[maybe_unused]] const std::vector<std::vector<FF>>& public_inputs)
{
using Flavor = {{name}}Flavor;
using FF = Flavor::FF;
Expand Down
Loading