fix(avm): correctly build spike vm (#7726)

After this PR you can now generate an alternative (spike) vm and it will build correctly. Most of the changes are to account for empty entity subsets. I had to add some typing information to `flavor_macros.hpp` to work when the flavor is empty. This is annoying but shouldn't have any runtime implications.
AztecProtocol · Aug 2, 2024 · 0c1d98f · 0c1d98f
1 parent e7c1dc3
commit 0c1d98f
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 21 deletions.
diff --git a/barretenberg/cpp/src/barretenberg/flavor/flavor_macros.hpp b/barretenberg/cpp/src/barretenberg/flavor/flavor_macros.hpp
@@ -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);
@@ -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__ };                        \
     }
 
 /**
@@ -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(...)                                                                                   \

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.cpp
@@ -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])

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp
@@ -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>
@@ -302,11 +304,12 @@ class AvmFlavor {
 
     class AllConstRefValues {
       public:
-        using DataType = const FF&;
+        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);
     };
 
     /**

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp
@@ -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;

diff --git a/bb-pilcom/bb-pil-backend/templates/flavor.cpp.hbs b/bb-pilcom/bb-pil-backend/templates/flavor.cpp.hbs
@@ -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}}

diff --git a/bb-pilcom/bb-pil-backend/templates/flavor.hpp.hbs b/bb-pilcom/bb-pil-backend/templates/flavor.hpp.hbs
@@ -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 {
@@ -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>
@@ -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() { 
@@ -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.
@@ -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);
       };
 
       /**

diff --git a/bb-pilcom/bb-pil-backend/templates/prover.cpp.hbs b/bb-pilcom/bb-pil-backend/templates/prover.cpp.hbs
@@ -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;

diff --git a/bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs b/bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs
@@ -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;