Skip to content

Commit

Permalink
chore: Parse negatives in SSA parser (noir-lang/noir#6510)
Browse files Browse the repository at this point in the history
feat: avoid unnecessary ssa passes while loop unrolling (noir-lang/noir#6509)
fix(tests): Use a file lock as well as a mutex to isolate tests cases (noir-lang/noir#6508)
fix: set local_module before elaborating each trait (noir-lang/noir#6506)
fix: parse Slice type in SSa (noir-lang/noir#6507)
fix: perform arithmetic simplification through `CheckedCast` (noir-lang/noir#6502)
feat: SSA parser (noir-lang/noir#6489)
chore(test): Run test matrix on test_programs (noir-lang/noir#6429)
chore(ci): fix cargo deny (noir-lang/noir#6501)
feat: Deduplicate instructions across blocks (noir-lang/noir#6499)
chore: move tests for arithmetic generics closer to the code (noir-lang/noir#6497)
fix(docs): Fix broken links in oracles doc (noir-lang/noir#6488)
fix: Treat all parameters as possible aliases of each other (noir-lang/noir#6477)
chore: bump rust dependencies (noir-lang/noir#6482)
feat: use a full `BlackBoxFunctionSolver` implementation when execution brillig during acirgen (noir-lang/noir#6481)
chore(docs): Update How to Oracles (noir-lang/noir#5675)
chore: Release Noir(0.38.0) (noir-lang/noir#6422)
fix(ssa): Change array_set to not mutate slices coming from function inputs (noir-lang/noir#6463)
chore: update example to show how to split public inputs in bash (noir-lang/noir#6472)
fix: Discard optimisation that would change execution ordering or that is related to call outputs (noir-lang/noir#6461)
chore: proptest for `canonicalize` on infix type expressions (noir-lang/noir#6269)
fix: let formatter respect newlines between comments (noir-lang/noir#6458)
fix: check infix expression is valid in program input (noir-lang/noir#6450)
fix: don't crash on AsTraitPath with empty path (noir-lang/noir#6454)
fix(tests): Prevent EOF error while running test programs (noir-lang/noir#6455)
fix(sea): mem2reg to treat block input references as alias (noir-lang/noir#6452)
chore: revamp attributes (noir-lang/noir#6424)
feat!: Always Check Arithmetic Generics at Monomorphization (noir-lang/noir#6329)
chore: split path and import lookups (noir-lang/noir#6430)
fix(ssa): Resolve value IDs in terminator before comparing to array (noir-lang/noir#6448)
fix: right shift is not a regular division (noir-lang/noir#6400)
  • Loading branch information
AztecBot committed Nov 14, 2024
2 parents 32e2fe6 + 70667ce commit fdededb
Show file tree
Hide file tree
Showing 19 changed files with 178 additions and 128 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1df8c456d6d256f120d6df6ae3e6735cb7eb7dae
0fc0c53ec183890370c69aa4148952b3123cb055
26 changes: 25 additions & 1 deletion 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
Expand Up @@ -295,6 +295,7 @@ mod tests {
public_parameters: PublicInputs::default(),
return_values: PublicInputs::default(),
assert_messages: Default::default(),
recursive: false,
};
check_circuit(circuit);
}
Expand Down Expand Up @@ -347,6 +348,7 @@ mod tests {
public_parameters: PublicInputs::default(),
return_values: PublicInputs::default(),
assert_messages: Default::default(),
recursive: false,
};
check_circuit(circuit);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ 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;
Expand All @@ -35,7 +34,7 @@ use super::{
/// Procedures are a set of complex operations that are common in the noir language.
/// Extracting them to reusable procedures allows us to reduce the size of the generated Brillig.
/// Procedures receive their arguments on scratch space to avoid stack dumping&restoring.
#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)]
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)]
pub enum ProcedureId {
ArrayCopy,
ArrayReverse,
Expand Down Expand Up @@ -160,6 +159,59 @@ impl std::fmt::Display for ProcedureId {
}
}

impl ProcedureId {
pub(crate) fn to_debug_id(self) -> ProcedureDebugId {
ProcedureDebugId(match self {
ProcedureId::ArrayCopy => 0,
ProcedureId::ArrayReverse => 1,
ProcedureId::VectorCopy => 2,
ProcedureId::MemCopy => 3,
ProcedureId::PrepareVectorPush(true) => 4,
ProcedureId::PrepareVectorPush(false) => 5,
ProcedureId::VectorPopFront => 6,
ProcedureId::VectorPopBack => 7,
ProcedureId::PrepareVectorInsert => 8,
ProcedureId::VectorRemove => 9,
ProcedureId::CheckMaxStackDepth => 10,
})
}

pub fn from_debug_id(debug_id: ProcedureDebugId) -> Self {
let inner = debug_id.0;
match inner {
0 => ProcedureId::ArrayCopy,
1 => ProcedureId::ArrayReverse,
2 => ProcedureId::VectorCopy,
3 => ProcedureId::MemCopy,
4 => ProcedureId::PrepareVectorPush(true),
5 => ProcedureId::PrepareVectorPush(false),
6 => ProcedureId::VectorPopFront,
7 => ProcedureId::VectorPopBack,
8 => ProcedureId::PrepareVectorInsert,
9 => ProcedureId::VectorRemove,
10 => ProcedureId::CheckMaxStackDepth,
_ => panic!("Unsupported procedure debug ID of {inner} was supplied"),
}
}
}

impl std::fmt::Display for ProcedureId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ProcedureId::ArrayCopy => write!(f, "ArrayCopy"),
ProcedureId::ArrayReverse => write!(f, "ArrayReverse"),
ProcedureId::VectorCopy => write!(f, "VectorCopy"),
ProcedureId::MemCopy => write!(f, "MemCopy"),
ProcedureId::PrepareVectorPush(flag) => write!(f, "PrepareVectorPush({flag})"),
ProcedureId::VectorPopFront => write!(f, "VectorPopFront"),
ProcedureId::VectorPopBack => write!(f, "VectorPopBack"),
ProcedureId::PrepareVectorInsert => write!(f, "PrepareVectorInsert"),
ProcedureId::VectorRemove => write!(f, "VectorRemove"),
ProcedureId::CheckMaxStackDepth => write!(f, "CheckMaxStackDepth"),
}
}
}

pub(crate) fn compile_procedure<F: AcirField + DebugToString>(
procedure_id: ProcedureId,
) -> BrilligArtifact<F> {
Expand Down
2 changes: 0 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ 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
/// how much leading whitespace there is on the first non-empty line.
#[cfg(test)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1010,13 +1010,13 @@ impl<'a> Context<'a> {
};
entry_point.link_with(artifact);
// Insert the range of opcode locations occupied by a procedure
if let Some(procedure_id) = &artifact.procedure {
if let Some(procedure_id) = artifact.procedure {
let num_opcodes = entry_point.byte_code.len();
let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
// We subtract one as to keep the range inclusive on both ends
entry_point
.procedure_locations
.insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
.insert(procedure_id, (previous_num_opcodes, num_opcodes - 1));
}
}
// Generate the final bytecode
Expand Down
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

This file was deleted.

This file was deleted.

Loading

0 comments on commit fdededb

Please sign in to comment.