Skip to content

Commit

Permalink
feat(bb): iterative constexpr_for (#8502)
Browse files Browse the repository at this point in the history
Replaces the custom made solution. Needed for stack not to blow up in a
few places.
  • Loading branch information
fcarreiro authored Sep 11, 2024
1 parent 5a76ec6 commit 02c3330
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 41 deletions.
51 changes: 20 additions & 31 deletions barretenberg/cpp/src/barretenberg/common/constexpr_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@
#include <utility>

namespace bb {
namespace detail {

/**
* @brief Create an index sequence from Min to Max (not included) with an increment of Inc
*/
template <size_t Min, size_t Max, size_t Inc> constexpr auto make_index_range()
{
static_assert(Max >= Min);
static_assert(Inc >= 1);
return []<size_t... Is>(std::index_sequence<Is...>) {
return std::index_sequence<Min + (Is * Inc)...>{};
}(std::make_index_sequence<(Max - Min - 1) / Inc + 1>{});
}

} // namespace detail

/**
* @brief Implements a loop using a compile-time iterator. Requires c++20.
Expand Down Expand Up @@ -55,37 +70,11 @@ namespace bb {
*/
template <size_t Start, size_t End, size_t Inc, class F> constexpr void constexpr_for(F&& f)
{
// Call function `f<Start>()` iff Start < End
if constexpr (Start < End) {
// F must be a template lambda with a single **typed** template parameter that represents the iterator
// (e.g. [&]<size_t i>(){ ... } is good)
// (and [&]<typename i>(){ ... } won't compile!)

/**
* Explaining f.template operator()<Start>()
*
* The following line must explicitly tell the compiler that <Start> is a template parameter by using the
* `template` keyword.
* (if we wrote f<Start>(), the compiler could legitimately interpret `<` as a less than symbol)
*
* The fragment `f.template` tells the compiler that we're calling a *templated* member of `f`.
* The "member" being called is the function operator, `operator()`, which must be explicitly provided
* (for any function X, `X(args)` is an alias for `X.operator()(args)`)
* The compiler has no alias `X.template <tparam>(args)` for `X.template operator()<tparam>(args)` so we must
* write it explicitly here
*
* To summarize what the next line tells the compiler...
* 1. I want to call a member of `f` that expects one or more template parameters
* 2. The member of `f` that I want to call is the function operator
* 3. The template parameter is `Start`
* 4. The function operator itself contains no arguments
*/
f.template operator()<Start>();

// Once we have executed `f`, we recursively call the `constexpr_for` function, increasing the value of `Start`
// by `Inc`
constexpr_for<Start + Inc, End, Inc>(f);
}
// F must be a template lambda with a single **typed** template parameter that represents the iterator
// (e.g. [&]<size_t i>(){ ... } is good)
// (and [&]<typename i>(){ ... } won't compile!)
constexpr auto indices = detail::make_index_range<Start, End, Inc>();
[&]<size_t... Is>(std::index_sequence<Is...>) { (f.template operator()<Is>(), ...); }(indices);
}

}; // namespace bb
12 changes: 2 additions & 10 deletions barretenberg/cpp/src/barretenberg/relations/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,6 @@ 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.
*
Expand All @@ -164,7 +156,7 @@ template <typename Flavor> class RelationUtils {
const Parameters& relation_parameters,
const FF& partial_evaluation_result)
{
iterative_constexpr_for<NUM_RELATIONS>([&]<size_t rel_index>() {
constexpr_for<0, NUM_RELATIONS, 1>([&]<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);
Expand All @@ -187,7 +179,7 @@ template <typename Flavor> class RelationUtils {
const Parameters& relation_parameters,
const FF& partial_evaluation_result)
{
iterative_constexpr_for<NUM_RELATIONS>([&]<size_t rel_index>() {
constexpr_for<0, NUM_RELATIONS, 1>([&]<size_t rel_index>() {
accumulate_single_relation<Parameters, rel_index>(
evaluations, relation_evaluations, relation_parameters, partial_evaluation_result);
});
Expand Down

0 comments on commit 02c3330

Please sign in to comment.