From 47e83fa680f46b12cd65c26475908987f97fff4d Mon Sep 17 00:00:00 2001
From: Facundo <fcarreiro@users.noreply.github.com>
Date: Tue, 27 Aug 2024 14:00:30 +0100
Subject: [PATCH] fix(bb): eliminate recursion in accumulate* (#8205)

Jean is working on the AVM recursive verifier and he found that these
functions were executed recursively (the compiler was indeed generating
recursive calls) and causing a stack overflow. This fixes that.
* ~~Also fixed `accumulate_relation_evaluations_without_skipping` which
was only not skipping the first relation.~~ Tests fail with the fix,
I've added a comment.
* I also made some params `const&`. IIUC they were being copied before
which can be massive for the type `AllValues`. Not sure about that but
you might want to check the callers, etc.
---
 .../cpp/src/barretenberg/relations/utils.hpp  | 52 ++++++++++++-------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/barretenberg/cpp/src/barretenberg/relations/utils.hpp b/barretenberg/cpp/src/barretenberg/relations/utils.hpp
index ba4da897156..681363124ad 100644
--- a/barretenberg/cpp/src/barretenberg/relations/utils.hpp
+++ b/barretenberg/cpp/src/barretenberg/relations/utils.hpp
@@ -3,6 +3,8 @@
 #include "barretenberg/polynomials/pow.hpp"
 #include "barretenberg/relations/relation_parameters.hpp"
 #include <tuple>
+#include <utility>
+
 namespace bb {
 
 template <typename Flavor> class RelationUtils {
@@ -138,6 +140,14 @@ template <typename Flavor> class RelationUtils {
         }
     }
 
+    // This is an simpler, iterative version of constexpr_for.
+    // Replace it with constexpr_for once that one is iterative.
+    template <size_t N, typename F> static constexpr void iterative_constexpr_for(F&& f)
+    {
+        auto seq = std::make_index_sequence<N>{};
+        [&]<size_t... I>(std::index_sequence<I...>) { (f.template operator()<I>(), ...); }(seq);
+    }
+
     /**
      * @brief Calculate the contribution of each relation to the expected value of the full Honk relation.
      *
@@ -147,23 +157,18 @@ template <typename Flavor> class RelationUtils {
      * relation. This value is checked against the final value of the target total sum (called sigma_0 in the
      * thesis).
      */
-    template <typename Parameters, size_t relation_idx = 0>
+    template <typename Parameters>
     // TODO(#224)(Cody): Input should be an array?
-    inline static void accumulate_relation_evaluations_without_skipping(PolynomialEvaluations evaluations,
+    inline static void accumulate_relation_evaluations_without_skipping(const PolynomialEvaluations& evaluations,
                                                                         RelationEvaluations& relation_evaluations,
                                                                         const Parameters& relation_parameters,
                                                                         const FF& partial_evaluation_result)
     {
-        using Relation = std::tuple_element_t<relation_idx, Relations>;
-
-        Relation::accumulate(
-            std::get<relation_idx>(relation_evaluations), evaluations, relation_parameters, partial_evaluation_result);
-
-        // Repeat for the next relation.
-        if constexpr (relation_idx + 1 < NUM_RELATIONS) {
-            accumulate_relation_evaluations<Parameters, relation_idx + 1>(
+        iterative_constexpr_for<NUM_RELATIONS>([&]<size_t rel_index>() {
+            // FIXME: You wan't /*consider_skipping=*/false here, but tests need to be fixed.
+            accumulate_single_relation<Parameters, rel_index, /*consider_skipping=*/true>(
                 evaluations, relation_evaluations, relation_parameters, partial_evaluation_result);
-        }
+        });
     }
 
     /**
@@ -175,17 +180,30 @@ template <typename Flavor> class RelationUtils {
      * relation. This value is checked against the final value of the target total sum (called sigma_0 in the
      * thesis).
      */
-    template <typename Parameters, size_t relation_idx = 0>
+    template <typename Parameters>
     // TODO(#224)(Cody): Input should be an array?
-    inline static void accumulate_relation_evaluations(PolynomialEvaluations evaluations,
+    inline static void accumulate_relation_evaluations(const PolynomialEvaluations& evaluations,
                                                        RelationEvaluations& relation_evaluations,
                                                        const Parameters& relation_parameters,
                                                        const FF& partial_evaluation_result)
+    {
+        iterative_constexpr_for<NUM_RELATIONS>([&]<size_t rel_index>() {
+            accumulate_single_relation<Parameters, rel_index>(
+                evaluations, relation_evaluations, relation_parameters, partial_evaluation_result);
+        });
+    }
+
+    template <typename Parameters, size_t relation_idx, bool consider_skipping = true>
+    inline static void accumulate_single_relation(const PolynomialEvaluations& evaluations,
+                                                  RelationEvaluations& relation_evaluations,
+                                                  const Parameters& relation_parameters,
+                                                  const FF& partial_evaluation_result)
     {
         using Relation = std::tuple_element_t<relation_idx, Relations>;
 
         // Check if the relation is skippable to speed up accumulation
-        if constexpr (!isSkippable<Relation, decltype(evaluations)> || !std::is_same_v<FF, bb::fr>) {
+        if constexpr (!consider_skipping || !isSkippable<Relation, decltype(evaluations)> ||
+                      !std::is_same_v<FF, bb::fr>) {
             // If not, accumulate normally
             Relation::accumulate(std::get<relation_idx>(relation_evaluations),
                                  evaluations,
@@ -200,12 +218,6 @@ template <typename Flavor> class RelationUtils {
                                      partial_evaluation_result);
             }
         }
-
-        // Repeat for the next relation.
-        if constexpr (relation_idx + 1 < NUM_RELATIONS) {
-            accumulate_relation_evaluations<Parameters, relation_idx + 1>(
-                evaluations, relation_evaluations, relation_parameters, partial_evaluation_result);
-        }
     }
 
     /**