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 with a stack and conditional inlining #8989

Merged
merged 43 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
c33685b
feat(avm): simulator relative addr
fcarreiro Sep 26, 2024
33e783a
feat: First implementation of a stack in brillig
sirasistant Sep 30, 2024
d9fb82d
make transpiler work altough wrongly
sirasistant Oct 1, 2024
288795a
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 1, 2024
a10965d
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 2, 2024
f2ef5d2
feat: add inline always annotations
sirasistant Oct 2, 2024
96b2e43
feat: conditional inlining
sirasistant Oct 3, 2024
2df690a
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 3, 2024
8e3c11a
add stack too deep check
sirasistant Oct 3, 2024
9354727
restore mov registers test
sirasistant Oct 3, 2024
14829f1
fix
sirasistant Oct 3, 2024
156114d
add test for stack auto compacting
sirasistant Oct 3, 2024
16af6d1
add test on relative addressing
sirasistant Oct 3, 2024
aadde7b
chore(avm)!: make indirects big enough for relative addressing
fcarreiro Oct 3, 2024
c1437c7
initial transpiler fixes
sirasistant Oct 3, 2024
3a30621
cpp side
fcarreiro Oct 3, 2024
1c08d8e
Merge branch 'fc/avm-big-enough-indirects' into arv/stack_implementation
sirasistant Oct 3, 2024
703e54b
fix
sirasistant Oct 3, 2024
0b142fa
remove ununsed operands
sirasistant Oct 3, 2024
9bd0874
fix(avm): MSM not including enough operands
fcarreiro Oct 3, 2024
33a90d8
update msm
sirasistant Oct 3, 2024
a153354
Merge branch 'fc/avm-fix-operands' into arv/stack_implementation
sirasistant Oct 3, 2024
1c8b907
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 4, 2024
ffd4add
fix some opcodes
sirasistant Oct 4, 2024
bb1d996
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 4, 2024
bfdf558
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 4, 2024
8663cda
comment changes
sirasistant Oct 4, 2024
8f7e84f
fix bad merge
sirasistant Oct 4, 2024
4b54943
fix unrolling
sirasistant Oct 4, 2024
2b63475
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 4, 2024
ba24b03
test: add test for inliner with different aggressiveness
sirasistant Oct 8, 2024
d713f37
use floats when aggregating weights
sirasistant Oct 8, 2024
ce9f1a3
refactor
sirasistant Oct 8, 2024
997f60c
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 8, 2024
c0f3fcd
oops!
sirasistant Oct 8, 2024
cb85533
inliner aggressiveness as i64
sirasistant Oct 8, 2024
ebdaed9
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 8, 2024
f466158
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 9, 2024
36bcb0f
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 9, 2024
f5fd1c3
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 9, 2024
801a6cb
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 10, 2024
14d527a
feat(avm): relative addressing in avm circuit (#9155)
jeanmon Oct 10, 2024
e0047f6
Merge branch 'master' into arv/stack_implementation
sirasistant Oct 10, 2024
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
11 changes: 2 additions & 9 deletions avm-transpiler/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,22 +144,15 @@ pub(crate) struct AddressingModeBuilder {
}

impl AddressingModeBuilder {
fn is_relative(&self, address: &MemoryAddress) -> bool {
match address {
MemoryAddress::Relative(_) => true,
MemoryAddress::Direct(_) => false,
}
}

pub(crate) fn direct_operand(mut self, address: &MemoryAddress) -> Self {
self.relative.push(self.is_relative(address));
self.relative.push(address.is_relative());
self.indirect.push(false);

self
}

pub(crate) fn indirect_operand(mut self, address: &MemoryAddress) -> Self {
self.relative.push(self.is_relative(address));
self.relative.push(address.is_relative());
self.indirect.push(true);

self
Expand Down
7 changes: 7 additions & 0 deletions noir/noir-repo/acvm-repo/brillig/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ impl MemoryAddress {
MemoryAddress::Relative(offset) => offset,
}
}

pub fn is_relative(&self) -> bool {
match self {
MemoryAddress::Relative(_) => true,
MemoryAddress::Direct(_) => false,
}
}
}

/// Describes the memory layout for an array/vector element
Expand Down
104 changes: 97 additions & 7 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ struct InlineContext {
/// the control flow graph has been flattened.
inline_no_predicates_functions: bool,

// We keep track of the recursive functions in the SSA to avoid inlining them in a brillig context.
recursive_functions: BTreeSet<FunctionId>,
// These are the functions of the program that we shouldn't inline.
functions_not_to_inline: BTreeSet<FunctionId>,
}

/// The per-function inlining context contains information that is only valid for one function.
Expand Down Expand Up @@ -261,7 +261,7 @@ fn should_retain_recursive(
return;
}

// We'll use some heuristics to decide wether to inline or not.
// We'll use some heuristics to decide whether to inline or not.
// We compute the weight (roughly the number of instructions) of the function after inlining
// And the interface cost of the function (the inherent cost at the callsite, roughly the number of args and returns)
// We then can compute an approximation of the cost of inlining vs the cost of retaining the function
Expand Down Expand Up @@ -359,7 +359,7 @@ impl InlineContext {
ssa: &Ssa,
entry_point: FunctionId,
inline_no_predicates_functions: bool,
recursive_functions: BTreeSet<FunctionId>,
functions_not_to_inline: BTreeSet<FunctionId>,
) -> InlineContext {
let source = &ssa.functions[&entry_point];
let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point);
Expand All @@ -370,7 +370,7 @@ impl InlineContext {
entry_point,
call_stack: CallStack::new(),
inline_no_predicates_functions,
recursive_functions,
functions_not_to_inline,
}
}

Expand Down Expand Up @@ -651,7 +651,7 @@ impl<'function> PerFunctionContext<'function> {
} else {
// If the called function is brillig, we inline only if it's into brillig and the function is not recursive
matches!(ssa.functions[&self.context.entry_point].runtime(), RuntimeType::Brillig(_))
&& !self.context.recursive_functions.contains(&called_func_id)
&& !self.context.functions_not_to_inline.contains(&called_func_id)
}
}

Expand Down Expand Up @@ -820,9 +820,10 @@ mod test {
function_builder::FunctionBuilder,
ir::{
basic_block::BasicBlockId,
function::RuntimeType,
instruction::{BinaryOp, Intrinsic, TerminatorInstruction},
map::Id,
types::Type,
types::{NumericType, Type},
},
};

Expand Down Expand Up @@ -1092,4 +1093,93 @@ mod test {
let main = ssa.main();
assert_eq!(main.reachable_blocks().len(), 4);
}

#[test]
fn inliner_disabled() {
// brillig fn foo {
// b0():
// v0 = call bar()
// return v0
// }
// brillig fn bar {
// b0():
// return 72
// }
let foo_id = Id::test_new(0);
let mut builder = FunctionBuilder::new("foo".into(), foo_id);
builder.set_runtime(RuntimeType::Brillig(InlineType::default()));

let bar_id = Id::test_new(1);
let bar = builder.import_function(bar_id);
let results = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec();
builder.terminate_with_return(results);

builder.new_brillig_function("bar".into(), bar_id, InlineType::default());
let expected_return = 72u128;
let seventy_two = builder.field_constant(expected_return);
builder.terminate_with_return(vec![seventy_two]);

let ssa = builder.finish();
assert_eq!(ssa.functions.len(), 2);

let inlined = ssa.inline_functions(f64::NEG_INFINITY);
// No inlining has happened
assert_eq!(inlined.functions.len(), 2);
}

#[test]
fn conditional_inlining() {
// In this example we call a larger abrillig function 3 times so the inliner refuses to inline the function.
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
// brillig fn foo {
// b0():
// v0 = call bar()
// v1 = call bar()
// v2 = call bar()
// return v0
// }
// brillig fn bar {
// b0():
// jmpif 1 then: b1, else: b2
// b1():
// jmp b3(Field 1)
// b3(v3: Field):
// return v3
// b2():
// jmp b3(Field 2)
// }
let foo_id = Id::test_new(0);
let mut builder = FunctionBuilder::new("foo".into(), foo_id);
builder.set_runtime(RuntimeType::Brillig(InlineType::default()));

let bar_id = Id::test_new(1);
let bar = builder.import_function(bar_id);
let v0 = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec();
let _v1 = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec();
let _v2 = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec();
builder.terminate_with_return(v0);

builder.new_brillig_function("bar".into(), bar_id, InlineType::default());
let bar_v0 =
builder.numeric_constant(1_usize, Type::Numeric(NumericType::Unsigned { bit_size: 1 }));
let then_block = builder.insert_block();
let else_block = builder.insert_block();
let join_block = builder.insert_block();
builder.terminate_with_jmpif(bar_v0, then_block, else_block);
builder.switch_to_block(then_block);
let one = builder.numeric_constant(FieldElement::one(), Type::field());
builder.terminate_with_jmp(join_block, vec![one]);
builder.switch_to_block(else_block);
let two = builder.numeric_constant(FieldElement::from(2_u128), Type::field());
builder.terminate_with_jmp(join_block, vec![two]);
let join_param = builder.add_block_parameter(join_block, Type::field());
builder.switch_to_block(join_block);
builder.terminate_with_return(vec![join_param]);

let ssa = builder.finish();
assert_eq!(ssa.functions.len(), 2);

let inlined = ssa.inline_functions(0f64);
// No inlining has happened
assert_eq!(inlined.functions.len(), 2);
}
}