From 0d5dcec5a16a927023b8d5fbbdf0d7e9b562c245 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 28 Aug 2023 20:27:06 +0000 Subject: [PATCH 1/2] don't optimize allocates in constant folding --- crates/noirc_evaluator/src/ssa.rs | 2 ++ .../src/ssa/opt/constant_folding.rs | 4 +++- .../src/hir/resolution/resolver.rs | 3 ++- crates/noirc_frontend/src/parser/parser.rs | 22 ++++++++----------- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs index 7dbd627a949..04170b07154 100644 --- a/crates/noirc_evaluator/src/ssa.rs +++ b/crates/noirc_evaluator/src/ssa.rs @@ -63,6 +63,8 @@ pub(crate) fn optimize_into_acir( // and this pass is missed, slice merging will fail inside of flattening. .mem2reg() .print(print_ssa_passes, "After Mem2Reg:") + .fold_constants() + .print(print_ssa_passes, "After Constant Folding:") .flatten_cfg() .print(print_ssa_passes, "After Flattening:") // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index e53bfac8c03..a3f96161626 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -102,7 +102,9 @@ impl Context { // If the instruction doesn't have side-effects, cache the results so we can reuse them if // the same instruction appears again later in the block. - if !instruction.has_side_effects(&function.dfg) { + if !instruction.has_side_effects(&function.dfg) + && !matches!(instruction, Instruction::Allocate) + { instruction_result_cache.insert(instruction, new_results.clone()); } for (old_result, new_result) in old_results.iter().zip(new_results) { diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index d0159474606..7cde9ae231d 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -371,7 +371,8 @@ impl<'a> Resolver<'a> { // expect() here is valid, because the only places we don't have a span are omitted types // e.g. a function without return type implicitly has a spanless UnresolvedType::Unit return type // To get an invalid env type, the user must explicitly specify the type, which will have a span - let env_span = env.span.expect("Unexpected missing span for closure environment type"); + let env_span = + env.span.expect("Unexpected missing span for closure environment type"); let env = Box::new(self.resolve_type_inner(*env, new_variables)); diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 5f4b5f5484a..1da476a42ee 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -40,8 +40,8 @@ use crate::{ BinaryOp, BinaryOpKind, BlockExpression, ConstrainStatement, Distinctness, FunctionDefinition, FunctionReturnType, Ident, IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, Path, PathKind, Pattern, Recoverable, - TraitConstraint, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnaryOp, - UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, TraitBound, + TraitBound, TraitConstraint, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnaryOp, + UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, }; use chumsky::prelude::*; @@ -527,19 +527,17 @@ fn trait_implementation_body() -> impl NoirParser> { } fn where_clause() -> impl NoirParser> { - struct MultiTraitConstraint { typ: UnresolvedType, trait_bounds: Vec, } - let constraints = parse_type() - .then_ignore(just(Token::Colon)) - .then(trait_bounds()) - .validate(|(typ, trait_bounds), span, emit| { + let constraints = parse_type().then_ignore(just(Token::Colon)).then(trait_bounds()).validate( + |(typ, trait_bounds), span, emit| { emit(ParserError::with_reason(ParserErrorReason::ExperimentalFeature("Traits"), span)); MultiTraitConstraint { typ, trait_bounds } - }); + }, + ); keyword(Keyword::Where) .ignore_then(constraints.separated_by(just(Token::Comma))) @@ -549,7 +547,8 @@ fn where_clause() -> impl NoirParser> { let mut result: Vec = Vec::new(); for constraint in x { for bound in constraint.trait_bounds { - result.push(TraitConstraint { typ:constraint.typ.clone(), trait_bound:bound } ); + result + .push(TraitConstraint { typ: constraint.typ.clone(), trait_bound: bound }); } } result @@ -557,10 +556,7 @@ fn where_clause() -> impl NoirParser> { } fn trait_bounds() -> impl NoirParser> { - trait_bound() - .separated_by(just(Token::Plus)) - .at_least(1) - .allow_trailing() + trait_bound().separated_by(just(Token::Plus)).at_least(1).allow_trailing() } fn trait_bound() -> impl NoirParser { From 0b11583e1d046421235c04b8a7178198ba2f58ad Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 28 Aug 2023 20:33:23 +0000 Subject: [PATCH 2/2] remove folding before flattening --- crates/noirc_evaluator/src/ssa.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs index 04170b07154..7dbd627a949 100644 --- a/crates/noirc_evaluator/src/ssa.rs +++ b/crates/noirc_evaluator/src/ssa.rs @@ -63,8 +63,6 @@ pub(crate) fn optimize_into_acir( // and this pass is missed, slice merging will fail inside of flattening. .mem2reg() .print(print_ssa_passes, "After Mem2Reg:") - .fold_constants() - .print(print_ssa_passes, "After Constant Folding:") .flatten_cfg() .print(print_ssa_passes, "After Flattening:") // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores