Skip to content

Commit 35e8ab7

Browse files
authored
chore: circuit to polys cleanup (#23013)
Two cleanups in trace-to-polynomials land. Originally motivated by small perf wins but is attractive for the code simplification: **1. Simpler permutation polynomial construction.** The old code built a large intermediate table and walked it to produce the sigma/id polynomials. The new code writes those polynomials directly in three steps: identity init, cycle linkages, public-input update. **2. Each block has only the non-gate selectors plus (at most) one gate selector.** Previously every block declared all eight or nine possible gate selectors using the hacky `ZeroSelector`. `ZeroSelector` class is gone plus lots of simplification in the block classes.
1 parent a2eae19 commit 35e8ab7

15 files changed

Lines changed: 355 additions & 1068 deletions

barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.hpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// =====================
66

77
#pragma once
8+
#include "barretenberg/honk/execution_trace/execution_trace_block.hpp"
89
#include <cstddef>
910
#include <cstdint>
1011
#include <functional>
@@ -13,6 +14,9 @@
1314

1415
namespace bb::gate_patterns {
1516

17+
using bb::GateKind;
18+
using bb::read_gate_selector;
19+
1620
enum class Wire : uint8_t {
1721
W_L,
1822
W_R,
@@ -415,11 +419,10 @@ inline const GatePattern DATABUS = { .name = "databus",
415419
// Helper functions
416420
// ============================================================================
417421

418-
template <typename Block, typename GateSelectorColumn>
419-
Selectors read_selectors(Block& block, size_t gate_index, const GateSelectorColumn& gate_selector_column)
422+
template <typename Block> Selectors read_selectors(Block& block, size_t gate_index, GateKind kind)
420423
{
421424
return Selectors{
422-
.gate_selector = static_cast<int64_t>(static_cast<uint64_t>(gate_selector_column[gate_index])),
425+
.gate_selector = static_cast<int64_t>(static_cast<uint64_t>(read_gate_selector(block, kind, gate_index))),
423426
.q_m_nz = !block.q_m()[gate_index].is_zero(),
424427
.q_1_nz = !block.q_1()[gate_index].is_zero(),
425428
.q_2_nz = !block.q_2()[gate_index].is_zero(),

barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.cpp

Lines changed: 38 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -66,22 +66,18 @@ inline void StaticAnalyzer_<FF, CircuitBuilder>::process_gate_variables(std::vec
6666
* @return Vector of real variable indices constrained by this gate
6767
*/
6868
template <typename FF, typename CircuitBuilder>
69-
template <typename Block, typename GateSelectorColumn>
69+
template <typename Block>
7070
std::vector<uint32_t> StaticAnalyzer_<FF, CircuitBuilder>::extract_gate_variables(
71-
size_t index,
72-
Block& blk,
73-
const bb::gate_patterns::GatePattern& pattern,
74-
const GateSelectorColumn& gate_selector_column)
71+
size_t index, Block& blk, const bb::gate_patterns::GatePattern& pattern, bb::GateKind kind)
7572
{
7673
using namespace bb::gate_patterns;
7774

78-
// Check if gate selector is active
79-
if (gate_selector_column[index].is_zero()) {
75+
if (read_gate_selector(blk, kind, index).is_zero()) {
8076
return {};
8177
}
8278

8379
// Read selectors and extract wire indices using the pattern
84-
Selectors selectors = read_selectors(blk, index, gate_selector_column);
80+
Selectors selectors = read_selectors(blk, index, kind);
8581
std::vector<uint32_t> gate_variables = extract_wires(blk, index, pattern, selectors);
8682

8783
// Convert to real indices and process
@@ -262,36 +258,36 @@ template <typename FF, typename CircuitBuilder> void StaticAnalyzer_<FF, Circuit
262258
for (size_t gate_idx = 0; gate_idx < blk.size(); gate_idx++) {
263259
// Try each pattern until one matches (returns non-empty)
264260
std::vector<uint32_t> cc;
265-
auto try_pattern = [&](const GatePattern& pattern, const auto& selector) {
261+
auto try_pattern = [&](const GatePattern& pattern, GateKind kind) {
266262
if (cc.empty()) {
267-
cc = extract_gate_variables(gate_idx, blk, pattern, selector);
263+
cc = extract_gate_variables(gate_idx, blk, pattern, kind);
268264
}
269265
};
270266

271267
// Standard gate patterns (mutually exclusive - at most one will match)
272-
try_pattern(ARITHMETIC, blk.q_arith());
273-
try_pattern(ELLIPTIC, blk.q_elliptic());
274-
try_pattern(LOOKUP, blk.q_lookup());
275-
try_pattern(POSEIDON2_EXTERNAL, blk.q_poseidon2_external());
268+
try_pattern(ARITHMETIC, GateKind::Arith);
269+
try_pattern(ELLIPTIC, GateKind::Elliptic);
270+
try_pattern(LOOKUP, GateKind::Lookup);
271+
try_pattern(POSEIDON2_EXTERNAL, GateKind::Poseidon2Ext);
276272
if constexpr (IsMegaBuilder<CircuitBuilder>) {
277-
try_pattern(POSEIDON2_QUAD_INTERNAL, blk.q_poseidon2_quad_internal());
278-
try_pattern(POSEIDON2_QUAD_INTERNAL_TERMINAL, blk.q_poseidon2_quad_internal_terminal());
279-
try_pattern(POSEIDON2_TRANSITION_ENTRY, blk.q_poseidon2_transition_entry());
280-
try_pattern(POSEIDON2_INITIAL_EXTERNAL, blk.q_poseidon2_external_initial());
273+
try_pattern(POSEIDON2_QUAD_INTERNAL, GateKind::Poseidon2QuadInt);
274+
try_pattern(POSEIDON2_QUAD_INTERNAL_TERMINAL, GateKind::Poseidon2QuadIntTerminal);
275+
try_pattern(POSEIDON2_TRANSITION_ENTRY, GateKind::Poseidon2TransitionEntry);
276+
try_pattern(POSEIDON2_INITIAL_EXTERNAL, GateKind::Poseidon2ExtInitial);
281277
} else {
282-
try_pattern(POSEIDON2_INTERNAL, blk.q_poseidon2_internal());
278+
try_pattern(POSEIDON2_INTERNAL, GateKind::Poseidon2Int);
283279
}
284-
try_pattern(NON_NATIVE_FIELD, blk.q_nnf());
285-
try_pattern(MEMORY, blk.q_memory()); // consistency gates only; access gates via ROM/RAM transcripts
286-
try_pattern(DELTA_RANGE, blk.q_delta_range());
280+
try_pattern(NON_NATIVE_FIELD, GateKind::Nnf);
281+
try_pattern(MEMORY, GateKind::Memory); // consistency gates only; access gates via ROM/RAM transcripts
282+
try_pattern(DELTA_RANGE, GateKind::DeltaRange);
287283

288284
if (!cc.empty() && connect_variables) {
289285
connect_all_variables_in_vector(cc);
290286
}
291287

292288
// MegaBuilder-specific patterns
293289
if constexpr (IsMegaBuilder<CircuitBuilder>) {
294-
auto databus_cc = extract_gate_variables(gate_idx, blk, DATABUS, blk.q_busread());
290+
auto databus_cc = extract_gate_variables(gate_idx, blk, DATABUS, GateKind::BusRead);
295291
if (!databus_cc.empty() && connect_variables) {
296292
connect_all_variables_in_vector(databus_cc);
297293
}
@@ -529,8 +525,8 @@ std::vector<ConnectedComponent> StaticAnalyzer_<FF, CircuitBuilder>::find_connec
529525
template <typename FF, typename CircuitBuilder>
530526
bool StaticAnalyzer_<FF, CircuitBuilder>::is_gate_sorted_rom(auto& memory_block, size_t gate_idx) const
531527
{
532-
return memory_block.q_memory()[gate_idx] == FF::one() && memory_block.q_1()[gate_idx] == FF::one() &&
533-
memory_block.q_2()[gate_idx] == FF::one();
528+
return memory_block.gate_selector_for(GateKind::Memory)[gate_idx] == FF::one() &&
529+
memory_block.q_1()[gate_idx] == FF::one() && memory_block.q_2()[gate_idx] == FF::one();
534530
}
535531

536532
/**
@@ -666,7 +662,7 @@ inline size_t StaticAnalyzer_<FF, CircuitBuilder>::process_current_decompose_cha
666662
if (out_idx != zero_idx) {
667663
variables_in_one_gate.erase(this->to_real(out_idx));
668664
}
669-
auto q_arith = arithmetic_block.q_arith()[current_index];
665+
auto q_arith = arithmetic_block.gate_selector_for(GateKind::Arith)[current_index];
670666
if (q_arith == 1 || current_index == arithmetic_block.size() - 1) {
671667
// this is the last gate in this chain, or we can't go next, so we have to stop a loop
672668
break;
@@ -1000,7 +996,7 @@ inline void StaticAnalyzer_<FF, CircuitBuilder>::remove_record_witness_variables
1000996
auto q_3 = memory_block.q_3()[gate_idx];
1001997
auto q_4 = memory_block.q_4()[gate_idx];
1002998
auto q_m = memory_block.q_m()[gate_idx];
1003-
auto q_arith = memory_block.q_arith()[gate_idx];
999+
auto q_arith = read_gate_selector(memory_block, GateKind::Arith, gate_idx);
10041000
if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() &&
10051001
q_arith.is_zero()) {
10061002
// record witness can be in both ROM and RAM gates, so we can ignore q_c
@@ -1125,7 +1121,7 @@ template <typename FF, typename CircuitBuilder> void StaticAnalyzer_<FF, Circuit
11251121
template <typename FF, typename CircuitBuilder>
11261122
void StaticAnalyzer_<FF, CircuitBuilder>::print_arithmetic_gate_info(size_t gate_index, auto& block)
11271123
{
1128-
auto q_arith = block.q_arith()[gate_index];
1124+
auto q_arith = read_gate_selector(block, GateKind::Arith, gate_index);
11291125
if (!q_arith.is_zero()) {
11301126
info("q_arith == ", q_arith);
11311127
// fisrtly, print selectors for standard plonk gate
@@ -1160,7 +1156,7 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_arithmetic_gate_info(size_t gate
11601156
template <typename FF, typename CircuitBuilder>
11611157
void StaticAnalyzer_<FF, CircuitBuilder>::print_elliptic_gate_info(size_t gate_index, auto& block)
11621158
{
1163-
auto q_elliptic = block.q_elliptic()[gate_index];
1159+
auto q_elliptic = read_gate_selector(block, GateKind::Elliptic, gate_index);
11641160
if (!q_elliptic.is_zero()) {
11651161
info("q_elliptic == ", q_elliptic);
11661162
info("q_1 == ", block.q_1()[gate_index]);
@@ -1193,7 +1189,7 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_elliptic_gate_info(size_t gate_i
11931189
template <typename FF, typename CircuitBuilder>
11941190
void StaticAnalyzer_<FF, CircuitBuilder>::print_plookup_gate_info(size_t gate_index, auto& block)
11951191
{
1196-
auto q_lookup = block.q_lookup()[gate_index];
1192+
auto q_lookup = read_gate_selector(block, GateKind::Lookup, gate_index);
11971193
if (!q_lookup.is_zero()) {
11981194
info("q_lookup == ", q_lookup);
11991195
auto q_2 = block.q_2()[gate_index];
@@ -1227,7 +1223,7 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_plookup_gate_info(size_t gate_in
12271223
template <typename FF, typename CircuitBuilder>
12281224
void StaticAnalyzer_<FF, CircuitBuilder>::print_delta_range_gate_info(size_t gate_index, auto& block)
12291225
{
1230-
auto q_delta_range = block.q_delta_range()[gate_index];
1226+
auto q_delta_range = read_gate_selector(block, GateKind::DeltaRange, gate_index);
12311227
if (!q_delta_range.is_zero()) {
12321228
info("q_delta_range == ", q_delta_range);
12331229
info("w_1 == ", block.w_l()[gate_index]);
@@ -1251,21 +1247,22 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_delta_range_gate_info(size_t gat
12511247
template <typename FF, typename CircuitBuilder>
12521248
void StaticAnalyzer_<FF, CircuitBuilder>::print_poseidon2s_gate_info(size_t gate_index, auto& block)
12531249
{
1254-
auto external_selector = block.q_poseidon2_external()[gate_index];
1250+
auto external_selector = read_gate_selector(block, GateKind::Poseidon2Ext, gate_index);
12551251
bool nonzero = !external_selector.is_zero();
12561252
if constexpr (IsMegaBuilder<CircuitBuilder>) {
1257-
nonzero = nonzero || !block.q_poseidon2_external_initial()[gate_index].is_zero() ||
1258-
!block.q_poseidon2_quad_internal()[gate_index].is_zero();
1253+
nonzero = nonzero || !read_gate_selector(block, GateKind::Poseidon2ExtInitial, gate_index).is_zero() ||
1254+
!read_gate_selector(block, GateKind::Poseidon2QuadInt, gate_index).is_zero();
12591255
} else {
1260-
nonzero = nonzero || !block.q_poseidon2_internal()[gate_index].is_zero();
1256+
nonzero = nonzero || !read_gate_selector(block, GateKind::Poseidon2Int, gate_index).is_zero();
12611257
}
12621258
if (nonzero) {
12631259
info("q_poseidon2_external == ", external_selector);
12641260
if constexpr (IsMegaBuilder<CircuitBuilder>) {
1265-
info("q_poseidon2_external_initial == ", block.q_poseidon2_external_initial()[gate_index]);
1266-
info("q_poseidon2_quad_internal == ", block.q_poseidon2_quad_internal()[gate_index]);
1261+
info("q_poseidon2_external_initial == ",
1262+
read_gate_selector(block, GateKind::Poseidon2ExtInitial, gate_index));
1263+
info("q_poseidon2_quad_internal == ", read_gate_selector(block, GateKind::Poseidon2QuadInt, gate_index));
12671264
} else {
1268-
info("q_poseidon2_internal == ", block.q_poseidon2_internal()[gate_index]);
1265+
info("q_poseidon2_internal == ", read_gate_selector(block, GateKind::Poseidon2Int, gate_index));
12691266
}
12701267
info("w_1 == ", block.w_l()[gate_index]);
12711268
info("w_2 == ", block.w_r()[gate_index]);
@@ -1291,7 +1288,7 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_poseidon2s_gate_info(size_t gate
12911288
template <typename FF, typename CircuitBuilder>
12921289
void StaticAnalyzer_<FF, CircuitBuilder>::print_nnf_gate_info(size_t gate_idx, auto& block)
12931290
{
1294-
auto q_nnf = block.q_nnf()[gate_idx];
1291+
auto q_nnf = read_gate_selector(block, GateKind::Nnf, gate_idx);
12951292
if (!q_nnf.is_zero()) {
12961293
info("q_nnf == ", q_nnf);
12971294
auto q_2 = block.q_2()[gate_idx];
@@ -1331,7 +1328,7 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_nnf_gate_info(size_t gate_idx, a
13311328
template <typename FF, typename CircuitBuilder>
13321329
void StaticAnalyzer_<FF, CircuitBuilder>::print_memory_gate_info(size_t gate_index, auto& block)
13331330
{
1334-
auto q_memory = block.q_memory()[gate_index];
1331+
auto q_memory = read_gate_selector(block, GateKind::Memory, gate_index);
13351332
if (!q_memory.is_zero()) {
13361333
info("q_memory == ", q_memory);
13371334
auto q_1 = block.q_1()[gate_index];
@@ -1395,7 +1392,7 @@ void StaticAnalyzer_<FF, CircuitBuilder>::print_variable_info(const uint32_t rea
13951392
print_nnf_gate_info(gate_index, block);
13961393
print_memory_gate_info(gate_index, block);
13971394
if constexpr (IsMegaBuilder<CircuitBuilder>) {
1398-
auto q_databus = block.q_busread()[gate_index];
1395+
auto q_databus = read_gate_selector(block, GateKind::BusRead, gate_index);
13991396
if (!q_databus.is_zero()) {
14001397
info("q_databus == ", q_databus);
14011398
}

barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ template <typename FF, typename CircuitBuilder> class StaticAnalyzer_ {
110110
/**
111111
* @brief Extract gate variables using a declarative pattern
112112
*/
113-
template <typename Block, typename GateSelectorColumn>
113+
template <typename Block>
114114
std::vector<uint32_t> extract_gate_variables(size_t index,
115115
Block& blk,
116116
const bb::gate_patterns::GatePattern& pattern,
117-
const GateSelectorColumn& gate_selector_column);
117+
bb::GateKind kind);
118118

119119
void process_execution_trace();
120120

barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -163,57 +163,6 @@ TEST(MegaCircuitBuilder, GoblinEccOpQueueUltraOps)
163163
}
164164
}
165165

166-
/**
167-
* @brief Check that the selector partitioning is correct for the mega circuit builder
168-
* @details We check that for the arithmetic, delta_range, elliptic, memory, nnf, lookup, busread, poseidon2_external
169-
* blocks, and the other selectors are zero on that block.
170-
*/
171-
TEST(MegaCircuitBuilder, CompleteSelectorPartitioningCheck)
172-
{
173-
auto builder = MegaCircuitBuilder();
174-
GoblinMockCircuits::construct_simple_circuit(builder);
175-
bool result = CircuitChecker::check(builder);
176-
EXPECT_EQ(result, true);
177-
178-
// For each block, we want to check that all of the other selectors are zero on that block besides the one
179-
// corresponding to the current block
180-
for (auto& block : builder.blocks.get()) {
181-
for (size_t i = 0; i < block.size(); ++i) {
182-
if (&block != &builder.blocks.arithmetic) {
183-
EXPECT_EQ(block.q_arith()[i], 0);
184-
}
185-
if (&block != &builder.blocks.delta_range) {
186-
EXPECT_EQ(block.q_delta_range()[i], 0);
187-
}
188-
if (&block != &builder.blocks.elliptic) {
189-
EXPECT_EQ(block.q_elliptic()[i], 0);
190-
}
191-
if (&block != &builder.blocks.memory) {
192-
EXPECT_EQ(block.q_memory()[i], 0);
193-
}
194-
if (&block != &builder.blocks.nnf) {
195-
EXPECT_EQ(block.q_nnf()[i], 0);
196-
}
197-
if (&block != &builder.blocks.lookup) {
198-
EXPECT_EQ(block.q_lookup()[i], 0);
199-
}
200-
if (&block != &builder.blocks.busread) {
201-
EXPECT_EQ(block.q_busread()[i], 0);
202-
}
203-
if (&block != &builder.blocks.poseidon2_external) {
204-
EXPECT_EQ(block.q_poseidon2_external()[i], 0);
205-
}
206-
// The Mega compressed Poseidon2 relations are selected only inside `poseidon2_quad_internal`;
207-
// all three selectors must be zero on every other block.
208-
if (&block != &builder.blocks.poseidon2_quad_internal) {
209-
EXPECT_EQ(block.q_poseidon2_quad_internal()[i], 0);
210-
EXPECT_EQ(block.q_poseidon2_quad_internal_terminal()[i], 0);
211-
EXPECT_EQ(block.q_poseidon2_transition_entry()[i], 0);
212-
}
213-
}
214-
}
215-
}
216-
217166
/**
218167
* @brief Verify that the ecc_op block is first in the trace and starts at offset 1 (after the zero row)
219168
*/

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder_lookup.test.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,10 @@ TEST_F(UltraCircuitBuilderLookup, StepSizeCoefficients)
7777
for (size_t i = 0; i < num_lookups; ++i) {
7878
const auto& table = builder.get_table(multi_table.basic_table_ids[i]);
7979
EXPECT_EQ(builder.blocks.lookup.q_3()[i], fr(table.table_index)); // unique table identifier
80-
EXPECT_EQ(builder.blocks.lookup.q_lookup()[i], fr(1)); // gate selector should be "on"
81-
EXPECT_EQ(builder.blocks.lookup.q_1()[i], fr(0)); // unused in lookup gates
82-
EXPECT_EQ(builder.blocks.lookup.q_4()[i], fr(0)); // unused in lookup gates
80+
EXPECT_EQ(builder.blocks.lookup.gate_selector_for(bb::GateKind::Lookup)[i],
81+
fr(1)); // gate selector should be "on"
82+
EXPECT_EQ(builder.blocks.lookup.q_1()[i], fr(0)); // unused in lookup gates
83+
EXPECT_EQ(builder.blocks.lookup.q_4()[i], fr(0)); // unused in lookup gates
8384
}
8485

8586
EXPECT_TRUE(CircuitChecker::check(builder));

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder_nonnative.test.cpp

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -257,41 +257,6 @@ TEST_F(UltraCircuitBuilderNonNative, MultiplicationLargeCarryRegression)
257257
EXPECT_EQ(builder.err(), "range_constrain_two_limbs: hi limb.");
258258
}
259259

260-
// Verifies that the NNF block only contains NNF gates (other selectors are zero)
261-
TEST_F(UltraCircuitBuilderNonNative, BlockSelectorIsolation)
262-
{
263-
UltraCircuitBuilder builder;
264-
265-
uint256_t a = uint256_t(fq::random_element());
266-
uint256_t b = uint256_t(fq::random_element());
267-
uint256_t modulus = fq::modulus;
268-
269-
auto [q, r] = compute_quotient_remainder(a, b, modulus);
270-
const auto [lo_1_idx, hi_1_idx] = create_non_native_multiplication(builder, a, b, q, r, modulus);
271-
272-
// Range check the carry (output) lo and hi limbs
273-
const bool is_low_70_bits = uint256_t(builder.get_variable(lo_1_idx)).get_msb() < 70;
274-
const bool is_high_70_bits = uint256_t(builder.get_variable(hi_1_idx)).get_msb() < 70;
275-
if (is_low_70_bits && is_high_70_bits) {
276-
builder.range_constrain_two_limbs(lo_1_idx, hi_1_idx, 70, 70);
277-
} else {
278-
builder.create_limbed_range_constraint(lo_1_idx, 72);
279-
builder.create_limbed_range_constraint(hi_1_idx, 72);
280-
}
281-
282-
EXPECT_TRUE(CircuitChecker::check(builder));
283-
284-
// Verify that in the NNF block, all non-NNF selectors are zero
285-
for (size_t i = 0; i < builder.blocks.nnf.size(); ++i) {
286-
EXPECT_EQ(builder.blocks.nnf.q_arith()[i], 0);
287-
EXPECT_EQ(builder.blocks.nnf.q_delta_range()[i], 0);
288-
EXPECT_EQ(builder.blocks.nnf.q_elliptic()[i], 0);
289-
EXPECT_EQ(builder.blocks.nnf.q_lookup()[i], 0);
290-
EXPECT_EQ(builder.blocks.nnf.q_poseidon2_external()[i], 0);
291-
EXPECT_EQ(builder.blocks.nnf.q_poseidon2_internal()[i], 0);
292-
}
293-
}
294-
295260
// Verifies non-native field addition with various scaling factors
296261
TEST_F(UltraCircuitBuilderNonNative, Addition)
297262
{

0 commit comments

Comments
 (0)