Skip to content

Commit

Permalink
chore(avm): separate some fixed tables (#7163)
Browse files Browse the repository at this point in the history
An experiment in separating fixed tables and (for now mannually)
creating a `merge_into` function to merge them into the main trace.

TODO:
* Avoid having to create dummy relations
* autogen merge_into
* apply concept to other non-fixed tables
  • Loading branch information
fcarreiro authored Jun 24, 2024
1 parent 6bf2b72 commit 1d4a9a2
Show file tree
Hide file tree
Showing 36 changed files with 469 additions and 316 deletions.
4 changes: 1 addition & 3 deletions barretenberg/cpp/pil/avm/binary.pil
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@

include "byte_lookup.pil";
include "main.pil";
include "fixed/byte_lookup.pil";

namespace binary(256);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

namespace byte_lookup(256);
// These columns are commited for now, but will be migrated to constant/fixed when
// we support more *exotic* code generation options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,9 @@ namespace gas(256);

// TODO(ISSUE_NUMBER): Constrain variable gas costs
pol commit l2_gas_fixed_table;
pol commit da_gas_fixed_table;
pol commit da_gas_fixed_table;

// DUMMY RELATIONS to force creation of hpp.
sel_gas_cost - sel_gas_cost = 0;
l2_gas_fixed_table - l2_gas_fixed_table = 0;
da_gas_fixed_table - da_gas_fixed_table = 0;
9 changes: 9 additions & 0 deletions barretenberg/cpp/pil/avm/fixed/powers.pil
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// This table should eventually be fixed.
// Contains 256 rows with the powers of 2 for 8-bit numbers.
// power_of_2 = 1 << clk;
namespace powers(256);
// clk will be the implicit power.
pol commit power_of_2;

// DUMMY RELATION to force creation of hpp.
power_of_2 - power_of_2 = 0;
10 changes: 4 additions & 6 deletions barretenberg/cpp/pil/avm/main.pil
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ include "alu.pil";
include "binary.pil";
include "constants.pil";
include "kernel.pil";
include "gas.pil";
include "fixed/gas.pil";
include "fixed/powers.pil";
include "gadgets/conversion.pil";
include "gadgets/sha256.pil";
include "gadgets/poseidon2.pil";
Expand Down Expand Up @@ -139,9 +140,6 @@ namespace main(256);
pol commit sel_rng_8; // Boolean selector for the 8-bit range check lookup
pol commit sel_rng_16; // Boolean selector for the 16-bit range check lookup

//===== Lookup table powers of 2 =============================================
pol commit table_pow_2; // Table of powers of 2 for 8-bit numbers.

//===== CONTROL FLOW ==========================================================
// Program counter
pol commit pc;
Expand Down Expand Up @@ -773,11 +771,11 @@ namespace main(256);

// Lookup for 2**(ib)
#[LOOKUP_POW_2_0]
alu.sel_shift_which {alu.ib, alu.two_pow_s} in sel_rng_8 {clk, table_pow_2};
alu.sel_shift_which {alu.ib, alu.two_pow_s} in sel_rng_8 {clk, powers.power_of_2};

// Lookup for 2**(t-ib)
#[LOOKUP_POW_2_1]
alu.sel_shift_which {alu.t_sub_s_bits , alu.two_pow_t_sub_s} in sel_rng_8 {clk, table_pow_2};
alu.sel_shift_which {alu.t_sub_s_bits , alu.two_pow_t_sub_s} in sel_rng_8 {clk, powers.power_of_2};

//====== Inter-table Constraints (Range Checks) ============================================
// TODO: Investigate optimising these range checks. Handling non-FF elements should require less range checks.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@
[[maybe_unused]] auto main_sel_rng_16 = View(new_term.main_sel_rng_16); \
[[maybe_unused]] auto main_sel_rng_8 = View(new_term.main_sel_rng_8); \
[[maybe_unused]] auto main_space_id = View(new_term.main_space_id); \
[[maybe_unused]] auto main_table_pow_2 = View(new_term.main_table_pow_2); \
[[maybe_unused]] auto main_tag_err = View(new_term.main_tag_err); \
[[maybe_unused]] auto main_w_in_tag = View(new_term.main_w_in_tag); \
[[maybe_unused]] auto mem_addr = View(new_term.mem_addr); \
Expand Down Expand Up @@ -293,6 +292,7 @@
[[maybe_unused]] auto poseidon2_input = View(new_term.poseidon2_input); \
[[maybe_unused]] auto poseidon2_output = View(new_term.poseidon2_output); \
[[maybe_unused]] auto poseidon2_sel_poseidon_perm = View(new_term.poseidon2_sel_poseidon_perm); \
[[maybe_unused]] auto powers_power_of_2 = View(new_term.powers_power_of_2); \
[[maybe_unused]] auto sha256_clk = View(new_term.sha256_clk); \
[[maybe_unused]] auto sha256_input = View(new_term.sha256_input); \
[[maybe_unused]] auto sha256_output = View(new_term.sha256_output); \
Expand Down
69 changes: 69 additions & 0 deletions barretenberg/cpp/src/barretenberg/relations/generated/avm/gas.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@

#pragma once
#include "../../relation_parameters.hpp"
#include "../../relation_types.hpp"
#include "./declare_views.hpp"

namespace bb::Avm_vm {

template <typename FF> struct GasRow {
FF gas_da_gas_fixed_table{};
FF gas_l2_gas_fixed_table{};
FF gas_sel_gas_cost{};

[[maybe_unused]] static std::vector<std::string> names();
};

inline std::string get_relation_label_gas(int index)
{
switch (index) {}
return std::to_string(index);
}

template <typename FF_> class gasImpl {
public:
using FF = FF_;

static constexpr std::array<size_t, 3> SUBRELATION_PARTIAL_LENGTHS{
2,
2,
2,
};

template <typename ContainerOverSubrelations, typename AllEntities>
void static accumulate(ContainerOverSubrelations& evals,
const AllEntities& new_term,
[[maybe_unused]] const RelationParameters<FF>&,
[[maybe_unused]] const FF& scaling_factor)
{

// Contribution 0
{
Avm_DECLARE_VIEWS(0);

auto tmp = (gas_sel_gas_cost - gas_sel_gas_cost);
tmp *= scaling_factor;
std::get<0>(evals) += tmp;
}
// Contribution 1
{
Avm_DECLARE_VIEWS(1);

auto tmp = (gas_l2_gas_fixed_table - gas_l2_gas_fixed_table);
tmp *= scaling_factor;
std::get<1>(evals) += tmp;
}
// Contribution 2
{
Avm_DECLARE_VIEWS(2);

auto tmp = (gas_da_gas_fixed_table - gas_da_gas_fixed_table);
tmp *= scaling_factor;
std::get<2>(evals) += tmp;
}
}
};

template <typename FF> using gas = Relation<gasImpl<FF>>;

} // namespace bb::Avm_vm
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class lookup_pow_2_0_lookup_settings {
in.alu_ib,
in.alu_two_pow_s,
in.main_clk,
in.main_table_pow_2);
in.powers_power_of_2);
}

/**
Expand All @@ -160,7 +160,7 @@ class lookup_pow_2_0_lookup_settings {
in.alu_ib,
in.alu_two_pow_s,
in.main_clk,
in.main_table_pow_2);
in.powers_power_of_2);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class lookup_pow_2_1_lookup_settings {
in.alu_t_sub_s_bits,
in.alu_two_pow_t_sub_s,
in.main_clk,
in.main_table_pow_2);
in.powers_power_of_2);
}

/**
Expand All @@ -160,7 +160,7 @@ class lookup_pow_2_1_lookup_settings {
in.alu_t_sub_s_bits,
in.alu_two_pow_t_sub_s,
in.main_clk,
in.main_table_pow_2);
in.powers_power_of_2);
}
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

#pragma once
#include "../../relation_parameters.hpp"
#include "../../relation_types.hpp"
#include "./declare_views.hpp"

namespace bb::Avm_vm {

template <typename FF> struct PowersRow {
FF powers_power_of_2{};

[[maybe_unused]] static std::vector<std::string> names();
};

inline std::string get_relation_label_powers(int index)
{
switch (index) {}
return std::to_string(index);
}

template <typename FF_> class powersImpl {
public:
using FF = FF_;

static constexpr std::array<size_t, 1> SUBRELATION_PARTIAL_LENGTHS{
2,
};

template <typename ContainerOverSubrelations, typename AllEntities>
void static accumulate(ContainerOverSubrelations& evals,
const AllEntities& new_term,
[[maybe_unused]] const RelationParameters<FF>&,
[[maybe_unused]] const FF& scaling_factor)
{

// Contribution 0
{
Avm_DECLARE_VIEWS(0);

auto tmp = (powers_power_of_2 - powers_power_of_2);
tmp *= scaling_factor;
std::get<0>(evals) += tmp;
}
}
};

template <typename FF> using powers = Relation<powersImpl<FF>>;

} // namespace bb::Avm_vm
15 changes: 11 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_gas_trace.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#include "barretenberg/vm/avm_trace/avm_gas_trace.hpp"

#include <cstddef>
#include <cstdint>

#include "barretenberg/vm/avm_trace/avm_opcode.hpp"
#include "barretenberg/vm/avm_trace/fixed_gas.hpp"

namespace bb::avm_trace {

Expand Down Expand Up @@ -39,8 +44,9 @@ void AvmGasTraceBuilder::constrain_gas_lookup(uint32_t clk, OpCode opcode)
gas_opcode_lookup_counter[opcode]++;

// Get the gas prices for this opcode
uint32_t l2_gas_cost = GAS_COST_TABLE.at(opcode).l2_fixed_gas_cost;
uint32_t da_gas_cost = GAS_COST_TABLE.at(opcode).da_fixed_gas_cost;
const auto& GAS_COST_TABLE = FixedGasTable::get();
auto l2_gas_cost = static_cast<uint32_t>(GAS_COST_TABLE.at(opcode).gas_l2_gas_fixed_table);
auto da_gas_cost = static_cast<uint32_t>(GAS_COST_TABLE.at(opcode).gas_da_gas_fixed_table);

remaining_l2_gas -= l2_gas_cost;
remaining_da_gas -= da_gas_cost;
Expand Down Expand Up @@ -69,8 +75,9 @@ void AvmGasTraceBuilder::constrain_gas_for_external_call(uint32_t clk,
// gas_opcode_lookup_counter[opcode]++;

// Get the gas prices for this opcode
uint32_t opcode_l2_gas_cost = GAS_COST_TABLE.at(opcode).l2_fixed_gas_cost;
uint32_t opcode_da_gas_cost = GAS_COST_TABLE.at(opcode).da_fixed_gas_cost;
const auto& GAS_COST_TABLE = FixedGasTable::get();
auto opcode_l2_gas_cost = static_cast<uint32_t>(GAS_COST_TABLE.at(opcode).gas_l2_gas_fixed_table);
auto opcode_da_gas_cost = static_cast<uint32_t>(GAS_COST_TABLE.at(opcode).gas_da_gas_fixed_table);

remaining_l2_gas -= opcode_l2_gas_cost + nested_l2_gas_cost;
remaining_da_gas -= opcode_da_gas_cost + nested_da_gas_cost;
Expand Down
Loading

1 comment on commit 1d4a9a2

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 1d4a9a2 Previous: 95546ea Ratio
Goblin::merge(t) 241080237 ns/iter 205391967 ns/iter 1.17

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.