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

chore: pull changes out of sync PR #9966

Merged
merged 5 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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: 11 additions & 0 deletions noir/noir-repo/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use noirc_errors::debug_info::ProcedureDebugId;
use serde::{Deserialize, Serialize};

mod array_copy;
mod array_reverse;
mod check_max_stack_depth;
Expand All @@ -14,11 +17,9 @@ use array_copy::compile_array_copy_procedure;
use array_reverse::compile_array_reverse_procedure;
use check_max_stack_depth::compile_check_max_stack_depth_procedure;
use mem_copy::compile_mem_copy_procedure;
use noirc_errors::debug_info::ProcedureDebugId;
use prepare_vector_insert::compile_prepare_vector_insert_procedure;
use prepare_vector_push::compile_prepare_vector_push_procedure;
use revert_with_string::compile_revert_with_string_procedure;
use serde::{Deserialize, Serialize};
use vector_copy::compile_vector_copy_procedure;
use vector_pop_back::compile_vector_pop_back_procedure;
use vector_pop_front::compile_vector_pop_front_procedure;
Expand Down
1 change: 0 additions & 1 deletion noir/noir-repo/compiler/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ pub mod ssa;
pub mod brillig;

pub use ssa::create_program;

pub use ssa::ir::instruction::ErrorType;

/// Trims leading whitespace from each line of the input string, according to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use acvm::acir::circuit::opcodes::{
};
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode};
use acvm::brillig_vm::{MemoryValue, VMStatus, VM};
use acvm::BlackBoxFunctionSolver;
use acvm::{
acir::AcirField,
acir::{
Expand Down Expand Up @@ -107,7 +108,9 @@ impl From<NumericType> for AcirType {
/// Context object which holds the relationship between
/// `Variables`(AcirVar) and types such as `Expression` and `Witness`
/// which are placed into ACIR.
pub(crate) struct AcirContext<F: AcirField> {
pub(crate) struct AcirContext<F: AcirField, B: BlackBoxFunctionSolver<F>> {
blackbox_solver: B,

/// Two-way map that links `AcirVar` to `AcirVarData`.
///
/// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`.
Expand All @@ -132,7 +135,7 @@ pub(crate) struct AcirContext<F: AcirField> {
pub(crate) warnings: Vec<SsaReport>,
}

impl<F: AcirField> AcirContext<F> {
impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
pub(crate) fn set_expression_width(&mut self, expression_width: ExpressionWidth) {
self.expression_width = expression_width;
}
Expand Down Expand Up @@ -1758,8 +1761,8 @@ impl<F: AcirField> AcirContext<F> {
brillig_stdlib_func,
);

fn range_constraint_value<G: AcirField>(
context: &mut AcirContext<G>,
fn range_constraint_value<G: AcirField, C: BlackBoxFunctionSolver<G>>(
context: &mut AcirContext<G, C>,
value: &AcirValue,
) -> Result<(), RuntimeError> {
match value {
Expand Down Expand Up @@ -1878,7 +1881,7 @@ impl<F: AcirField> AcirContext<F> {
inputs: &[BrilligInputs<F>],
outputs_types: &[AcirType],
) -> Option<Vec<AcirValue>> {
let mut memory = (execute_brillig(code, inputs)?).into_iter();
let mut memory = (execute_brillig(code, &self.blackbox_solver, inputs)?).into_iter();

let outputs_var = vecmap(outputs_types.iter(), |output| match output {
AcirType::NumericType(_) => {
Expand Down Expand Up @@ -2171,8 +2174,9 @@ pub(crate) struct AcirVar(usize);
/// Returns the finished state of the Brillig VM if execution can complete.
///
/// Returns `None` if complete execution of the Brillig bytecode is not possible.
fn execute_brillig<F: AcirField>(
fn execute_brillig<F: AcirField, B: BlackBoxFunctionSolver<F>>(
code: &[BrilligOpcode<F>],
blackbox_solver: &B,
inputs: &[BrilligInputs<F>],
) -> Option<Vec<MemoryValue<F>>> {
// Set input values
Expand All @@ -2198,12 +2202,8 @@ fn execute_brillig<F: AcirField>(
}

// Instantiate a Brillig VM given the solved input registers and memory, along with the Brillig bytecode.
//
// We pass a stubbed solver here as a concrete solver implies a field choice which conflicts with this function
// being generic.
let solver = acvm::blackbox_solver::StubbedBlackBoxSolver;
let profiling_active = false;
let mut vm = VM::new(calldata, code, Vec::new(), &solver, profiling_active);
let mut vm = VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active);

// Run the Brillig VM on these inputs, bytecode, etc!
let vm_status = vm.process_opcodes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunction
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
pub(crate) use acir_ir::generated_acir::GeneratedAcir;
use acvm::acir::circuit::opcodes::{AcirFunctionId, BlockType};
use bn254_blackbox_solver::Bn254BlackBoxSolver;
use noirc_frontend::monomorphization::ast::InlineType;

use acvm::acir::circuit::brillig::{BrilligBytecode, BrilligFunctionId};
Expand Down Expand Up @@ -157,7 +158,7 @@ struct Context<'a> {
current_side_effects_enabled_var: AcirVar,

/// Manages and builds the `AcirVar`s to which the converted SSA values refer.
acir_context: AcirContext<FieldElement>,
acir_context: AcirContext<FieldElement, Bn254BlackBoxSolver>,

/// Track initialized acir dynamic arrays
///
Expand Down
4 changes: 2 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub(crate) fn assert_normalized_ssa_equals(mut ssa: super::Ssa, expected: &str)
let expected = trim_leading_whitespace_from_lines(expected);

if ssa != expected {
println!("Got:\n~~~\n{}\n~~~\nExpected:\n~~~\n{}\n~~~", ssa, expected);
similar_asserts::assert_eq!(ssa, expected);
println!("Expected:\n~~~\n{expected}\n~~~\nGot:\n~~~\n{ssa}\n~~~");
similar_asserts::assert_eq!(expected, ssa);
}
}
98 changes: 53 additions & 45 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,54 +40,47 @@ use fxhash::FxHashMap as HashMap;
impl Ssa {
/// Loop unrolling can return errors, since ACIR functions need to be fully unrolled.
/// This meta-pass will keep trying to unroll loops and simplifying the SSA until no more errors are found.
#[tracing::instrument(level = "trace", skip(ssa))]
pub(crate) fn unroll_loops_iteratively(mut ssa: Ssa) -> Result<Ssa, RuntimeError> {
// Try to unroll loops first:
let mut unroll_errors;
(ssa, unroll_errors) = ssa.try_to_unroll_loops();

// Keep unrolling until no more errors are found
while !unroll_errors.is_empty() {
let prev_unroll_err_count = unroll_errors.len();

// Simplify the SSA before retrying

// Do a mem2reg after the last unroll to aid simplify_cfg
ssa = ssa.mem2reg();
ssa = ssa.simplify_cfg();
// Do another mem2reg after simplify_cfg to aid the next unroll
ssa = ssa.mem2reg();

// Unroll again
(ssa, unroll_errors) = ssa.try_to_unroll_loops();
// If we didn't manage to unroll any more loops, exit
if unroll_errors.len() >= prev_unroll_err_count {
return Err(unroll_errors.swap_remove(0));
let acir_functions = ssa.functions.iter_mut().filter(|(_, func)| {
// Loop unrolling in brillig can lead to a code explosion currently. This can
// also be true for ACIR, but we have no alternative to unrolling in ACIR.
// Brillig also generally prefers smaller code rather than faster code.
!matches!(func.runtime(), RuntimeType::Brillig(_))
});

for (_, function) in acir_functions {
// Try to unroll loops first:
let mut unroll_errors = function.try_to_unroll_loops();

// Keep unrolling until no more errors are found
while !unroll_errors.is_empty() {
let prev_unroll_err_count = unroll_errors.len();

// Simplify the SSA before retrying

// Do a mem2reg after the last unroll to aid simplify_cfg
function.mem2reg();
function.simplify_function();
// Do another mem2reg after simplify_cfg to aid the next unroll
function.mem2reg();

// Unroll again
unroll_errors = function.try_to_unroll_loops();
// If we didn't manage to unroll any more loops, exit
if unroll_errors.len() >= prev_unroll_err_count {
return Err(unroll_errors.swap_remove(0));
}
}
}
Ok(ssa)
}

/// Tries to unroll all loops in each SSA function.
/// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state.
/// Returns the ssa along with all unrolling errors encountered
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn try_to_unroll_loops(mut self) -> (Ssa, Vec<RuntimeError>) {
let mut errors = vec![];
for function in self.functions.values_mut() {
function.try_to_unroll_loops(&mut errors);
}
(self, errors)
Ok(ssa)
}
}

impl Function {
pub(crate) fn try_to_unroll_loops(&mut self, errors: &mut Vec<RuntimeError>) {
// Loop unrolling in brillig can lead to a code explosion currently. This can
// also be true for ACIR, but we have no alternative to unrolling in ACIR.
// Brillig also generally prefers smaller code rather than faster code.
if !matches!(self.runtime(), RuntimeType::Brillig(_)) {
errors.extend(find_all_loops(self).unroll_each_loop(self));
}
fn try_to_unroll_loops(&mut self) -> Vec<RuntimeError> {
find_all_loops(self).unroll_each_loop(self)
}
}

Expand Down Expand Up @@ -507,11 +500,26 @@ impl<'f> LoopIteration<'f> {

#[cfg(test)]
mod tests {
use crate::ssa::{
function_builder::FunctionBuilder,
ir::{instruction::BinaryOp, map::Id, types::Type},
use crate::{
errors::RuntimeError,
ssa::{
function_builder::FunctionBuilder,
ir::{instruction::BinaryOp, map::Id, types::Type},
},
};

use super::Ssa;

/// Tries to unroll all loops in each SSA function.
/// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state.
fn try_to_unroll_loops(mut ssa: Ssa) -> (Ssa, Vec<RuntimeError>) {
let mut errors = vec![];
for function in ssa.functions.values_mut() {
errors.extend(function.try_to_unroll_loops());
}
(ssa, errors)
}

#[test]
fn unroll_nested_loops() {
// fn main() {
Expand Down Expand Up @@ -630,7 +638,7 @@ mod tests {
// }
// The final block count is not 1 because unrolling creates some unnecessary jmps.
// If a simplify cfg pass is ran afterward, the expected block count will be 1.
let (ssa, errors) = ssa.try_to_unroll_loops();
let (ssa, errors) = try_to_unroll_loops(ssa);
assert_eq!(errors.len(), 0, "All loops should be unrolled");
assert_eq!(ssa.main().reachable_blocks().len(), 5);
}
Expand Down Expand Up @@ -680,7 +688,7 @@ mod tests {
assert_eq!(ssa.main().reachable_blocks().len(), 4);

// Expected that we failed to unroll the loop
let (_, errors) = ssa.try_to_unroll_loops();
let (_, errors) = try_to_unroll_loops(ssa);
assert_eq!(errors.len(), 1, "Expected to fail to unroll loop");
}
}
9 changes: 8 additions & 1 deletion noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,10 +735,17 @@ impl<'a> Parser<'a> {
}

fn eat_int(&mut self) -> ParseResult<Option<FieldElement>> {
let negative = self.eat(Token::Dash)?;

if matches!(self.token.token(), Token::Int(..)) {
let token = self.bump()?;
match token.into_token() {
Token::Int(int) => Ok(Some(int)),
Token::Int(mut int) => {
if negative {
int = -int;
}
Ok(Some(int))
}
_ => unreachable!(),
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl<'a> Lexer<'a> {
Some(']') => self.single_char_token(Token::RightBracket),
Some('&') => self.single_char_token(Token::Ampersand),
Some('-') if self.peek_char() == Some('>') => self.double_char_token(Token::Arrow),
Some('-') => self.single_char_token(Token::Dash),
Some(ch) if ch.is_ascii_alphanumeric() || ch == '_' => self.eat_alpha_numeric(ch),
Some(char) => Err(LexerError::UnexpectedCharacter {
char,
Expand Down
11 changes: 11 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,3 +425,14 @@ fn test_slice() {
";
assert_ssa_roundtrip(src);
}

#[test]
fn test_negative() {
let src = "
acir(inline) fn main f0 {
b0():
return Field -1
}
";
assert_ssa_roundtrip(src);
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub(crate) enum Token {
Equal,
/// &
Ampersand,
/// -
Dash,
Eof,
}

Expand Down Expand Up @@ -90,6 +92,7 @@ impl Display for Token {
Token::Arrow => write!(f, "->"),
Token::Equal => write!(f, "=="),
Token::Ampersand => write!(f, "&"),
Token::Dash => write!(f, "-"),
Token::Eof => write!(f, "(end of stream)"),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,14 @@ impl<'context> Elaborator<'context> {
}
ItemKind::Impl(r#impl) => {
let module = self.module_id();
dc_mod::collect_impl(self.interner, generated_items, r#impl, self.file, module);
dc_mod::collect_impl(
self.interner,
generated_items,
r#impl,
self.file,
module,
&mut self.errors,
);
}

ItemKind::ModuleDecl(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::{
};

use self::builtin_helpers::{eq_item, get_array, get_ctstring, get_str, get_u8, hash_item, lex};
use super::Interpreter;
use super::{foreign, Interpreter};

pub(crate) mod builtin_helpers;

Expand All @@ -57,6 +57,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
let interner = &mut self.elaborator.interner;
let call_stack = &self.elaborator.interpreter_call_stack;
match name {
"apply_range_constraint" => foreign::apply_range_constraint(arguments, location),
"array_as_str_unchecked" => array_as_str_unchecked(interner, arguments, location),
"array_len" => array_len(interner, arguments, location),
"assert_constant" => Ok(Value::Bool(true)),
Expand Down
Loading
Loading