Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(brillig): Hoist shared constants across functions to the global space #7216

Merged
merged 27 commits into from
Feb 18, 2025
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3521ec5
initial work to hoist shared constants across function to the global …
vezenovm Jan 28, 2025
3c2e0e9
cleanup
vezenovm Jan 28, 2025
da763fa
selectively hoist based off num occurrences
vezenovm Jan 28, 2025
c2c1370
use constants not consts
vezenovm Jan 29, 2025
bf5a35a
merge conflicts w/ master but no longer hoisting after entry point an…
vezenovm Jan 29, 2025
a6d631c
updated hoisting to work per entry point
vezenovm Jan 29, 2025
637b157
fix brillig globals unit tests
vezenovm Jan 30, 2025
0993511
fixup brillig context structure
vezenovm Jan 30, 2025
bd0473e
helper for checking if we have hoisted global when compiling a block
vezenovm Jan 30, 2025
6fa2d05
Apply suggestions from code review
vezenovm Jan 30, 2025
381e0f3
Merge branch 'master' into mv/hoist-shared-consts-globally
TomAFrench Jan 30, 2025
e0ccaa7
Apply suggestions from code review
vezenovm Jan 30, 2025
0f7940c
Update compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block…
vezenovm Jan 30, 2025
eb3b3c1
Apply suggestions from code review
vezenovm Jan 30, 2025
631a655
unit tests for shared constant hoisting
vezenovm Jan 30, 2025
1d68b7d
cmments on globals artifact type
vezenovm Jan 30, 2025
40c3b5b
split up long line
vezenovm Jan 30, 2025
2e5e5d7
use entry_points[0] not brillig_function_id
vezenovm Jan 30, 2025
1630e31
Merge branch 'master' into mv/hoist-shared-consts-globally
vezenovm Jan 30, 2025
2940cc3
Merge branch 'master' into mv/hoist-shared-consts-globally
vezenovm Jan 31, 2025
871811d
merge conflicts w/ master
vezenovm Feb 3, 2025
5fb5cbf
merge conflicts w/ master and cleanup
vezenovm Feb 18, 2025
c739199
nit cleanup
vezenovm Feb 18, 2025
8c8bf04
Update compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
vezenovm Feb 18, 2025
d54d418
Update compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
vezenovm Feb 18, 2025
b7a47af
missing &
vezenovm Feb 18, 2025
86986e5
Merge remote-tracking branch 'origin/mv/hoist-shared-consts-globally'…
vezenovm Feb 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
use entry_points[0] not brillig_function_id
vezenovm committed Jan 30, 2025
commit 2e5e5d71fb95c9c1ca1f4e3ead1df8570552c922
Original file line number Diff line number Diff line change
@@ -270,7 +270,7 @@ impl BrilligGlobals {
// Thus, if a call can be used by two entry points we should not use the hoisted constant allocations.
if entry_points.len() == 1 {
let hoisted_allocations =
self.entry_point_hoisted_globals_map.get(&brillig_function_id);
self.entry_point_hoisted_globals_map.get(&entry_points[0]);
let hoisted_allocations = hoisted_allocations.unwrap_or_else(|| panic!("ICE: Expected hoisted allocations to be set for function {brillig_function_id}"));
hoisted_constants_allocations.extend(hoisted_allocations);
}

Unchanged files with check annotations Beta

}
for (i, bit_size) in arguments.iter().flat_map(flat_bit_sizes).enumerate() {
// Calldatacopy tags everything with field type, so when downcast when necessary

Check warning on line 182 in compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs

GitHub Actions / Code

Unknown word (Calldatacopy)
if bit_size < F::max_num_bits() {
self.cast_instruction(
SingleAddrVariable::new(
// These can fail.
Constrain(..) | ConstrainNotEqual(..) | RangeCheck { .. } => true,
// This should never be side-effectful

Check warning on line 426 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

GitHub Actions / Code

Unknown word (effectful)
MakeArray { .. } | Noop => false,
// Some binary math can overflow or underflow
Or,
/// Bitwise xor (^)
Xor,
/// Bitshift left (<<)

Check warning on line 42 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

GitHub Actions / Code

Unknown word (Bitshift)
Shl,
/// Bitshift right (>>)

Check warning on line 44 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

GitHub Actions / Code

Unknown word (Bitshift)
Shr,
}
}
let operator = if lhs_type == NumericType::NativeField {
// Unchecked operations between fields or bools don't make sense, so we convert those to non-unchecked

Check warning on line 110 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

GitHub Actions / Code

Unknown word (bools)
// to reduce noise and confusion in the generated SSA.
match operator {
BinaryOp::Add { unchecked: true } => BinaryOp::Add { unchecked: false },
_ => operator,
}
} else if lhs_type == NumericType::bool() {
// Unchecked mul between bools doesn't make sense, so we convert that to non-unchecked

Check warning on line 119 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

GitHub Actions / Code

Unknown word (bools)
if let BinaryOp::Mul { unchecked: true } = operator {
BinaryOp::Mul { unchecked: false }
} else {
Instruction::binary(BinaryOp::Mul { unchecked: true }, lhs, rhs);
return SimplifyResult::SimplifiedToInstruction(instruction);
}
if lhs_type.is_unsigned() {

Check warning on line 292 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

GitHub Actions / Code

Unknown word (bools)
// It's common in other programming languages to truncate values to a certain bit size using
// a bitwise AND with a bit mask. However this operation is quite inefficient inside a snark.
//
// Set of functions we call which the purity result depends on.
// `is_pure` is intended to be called on each function, building
// up a call graph of sorts to check afterwards to propagate impurity
// from called functions to their callers. Resultingly, an initial "Pure"

Check warning on line 116 in compiler/noirc_evaluator/src/ssa/opt/pure.rs

GitHub Actions / Code

Unknown word (Resultingly)
// result here could be overridden by one of these dependencies being impure.
let mut dependencies = BTreeSet::new();
use super::{is_new_size_ok, BoilerplateStats, Loops};
/// Tries to unroll all loops in each SSA function once, calling the `Function` directly,
/// bypassing the iterative loop done by the SSA which does further optimisations.

Check warning on line 1070 in compiler/noirc_evaluator/src/ssa/opt/unrolling.rs

GitHub Actions / Code

Unknown word (optimisations)
///
/// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state.
fn try_unroll_loops(mut ssa: Ssa) -> (Ssa, Vec<RuntimeError>) {
/// = bool
/// | int
/// | str
/// | rawstr

Check warning on line 604 in compiler/noirc_frontend/src/parser/parser/expression.rs

GitHub Actions / Code

Unknown word (rawstr)
/// | fmtstr
/// | QuoteExpression
/// | ArrayExpression