diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index e76429b955b..acaaa163476 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -212,7 +212,8 @@ impl Context { if let Value::Intrinsic(intrinsic) = context.dfg[*func] { let results = context.dfg.instruction_results(instruction_id); - match slice_capacity_change(context.dfg, intrinsic, arguments, results) { + match self.slice_capacity_change(context.dfg, intrinsic, arguments, results) + { SizeChange::None => (), SizeChange::SetTo { old, new } => { self.set_capacity(context.dfg, old, new, |c| c); @@ -280,6 +281,101 @@ impl Context { } } } + + /// Find the change to a slice's capacity an instruction would have + fn slice_capacity_change( + &self, + dfg: &DataFlowGraph, + intrinsic: Intrinsic, + arguments: &[ValueId], + results: &[ValueId], + ) -> SizeChange { + match intrinsic { + Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SliceInsert => { + // All of these return `Self` (the slice), we are expecting: len, slice = ... + assert_eq!(results.len(), 2); + let old = arguments[1]; + let new = results[1]; + assert!(matches!(dfg.type_of_value(old), Type::Slice(_))); + assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); + SizeChange::Inc { old, new } + } + + Intrinsic::SlicePopBack | Intrinsic::SliceRemove => { + // fn pop_back(self) -> (Self, T) + // fn remove(self, index: u32) -> (Self, T) + // + // These functions return the slice as the result `(len, slice, ...item)`, + // so the slice is the second result. + let old = arguments[1]; + let new = results[1]; + assert!(matches!(dfg.type_of_value(old), Type::Slice(_))); + assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); + SizeChange::Dec { old, new } + } + + Intrinsic::SlicePopFront => { + // fn pop_front(self) -> (T, Self) + // + // These functions return the slice as the result `(...item, len, slice)`, + // so the slice is the last result. + let old = arguments[1]; + let new = results[results.len() - 1]; + assert!(matches!(dfg.type_of_value(old), Type::Slice(_))); + assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); + SizeChange::Dec { old, new } + } + + Intrinsic::AsSlice => { + assert_eq!(arguments.len(), 1); + assert_eq!(results.len(), 2); + let old = arguments[0]; + let new = results[1]; + assert!(matches!(dfg.type_of_value(old), Type::Array(_, _))); + assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); + SizeChange::SetTo { old, new } + } + + Intrinsic::Hint(Hint::BlackBox) => { + assert_eq!(arguments.len(), results.len()); + let arguments_types = + arguments.iter().map(|x| dfg.type_of_value(*x)).collect::>(); + let results_types = + results.iter().map(|x| dfg.type_of_value(*x)).collect::>(); + assert_eq!(arguments_types, results_types); + let old = + *arguments.last().expect("expected at least one argument to Hint::BlackBox"); + if self.slice_sizes.contains_key(&old) { + if arguments.len() != 1 { + assert!(arguments.len() == 2); + assert!(matches!(arguments_types[0], Type::Numeric(_))); + } + assert!(matches!(arguments_types.last().unwrap(), Type::Slice(_))); + let new = *results.last().unwrap(); + SizeChange::SetTo { old, new } + } else { + SizeChange::None + } + } + + // These cases don't affect slice capacities + Intrinsic::AssertConstant + | Intrinsic::StaticAssert + | Intrinsic::ApplyRangeConstraint + | Intrinsic::ArrayLen + | Intrinsic::ArrayAsStrUnchecked + | Intrinsic::StrAsBytes + | Intrinsic::BlackBox(_) + | Intrinsic::AsWitness + | Intrinsic::IsUnconstrained + | Intrinsic::DerivePedersenGenerators + | Intrinsic::ToBits(_) + | Intrinsic::ToRadix(_) + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount + | Intrinsic::FieldLessThan => SizeChange::None, + } + } } enum SizeChange { @@ -301,79 +397,6 @@ enum SizeChange { }, } -/// Find the change to a slice's capacity an instruction would have -fn slice_capacity_change( - dfg: &DataFlowGraph, - intrinsic: Intrinsic, - arguments: &[ValueId], - results: &[ValueId], -) -> SizeChange { - match intrinsic { - Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SliceInsert => { - // All of these return `Self` (the slice), we are expecting: len, slice = ... - assert_eq!(results.len(), 2); - let old = arguments[1]; - let new = results[1]; - assert!(matches!(dfg.type_of_value(old), Type::Slice(_))); - assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); - SizeChange::Inc { old, new } - } - - Intrinsic::SlicePopBack | Intrinsic::SliceRemove => { - // fn pop_back(self) -> (Self, T) - // fn remove(self, index: u32) -> (Self, T) - // - // These functions return the slice as the result `(len, slice, ...item)`, - // so the slice is the second result. - let old = arguments[1]; - let new = results[1]; - assert!(matches!(dfg.type_of_value(old), Type::Slice(_))); - assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); - SizeChange::Dec { old, new } - } - - Intrinsic::SlicePopFront => { - // fn pop_front(self) -> (T, Self) - // - // These functions return the slice as the result `(...item, len, slice)`, - // so the slice is the last result. - let old = arguments[1]; - let new = results[results.len() - 1]; - assert!(matches!(dfg.type_of_value(old), Type::Slice(_))); - assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); - SizeChange::Dec { old, new } - } - - Intrinsic::AsSlice => { - assert_eq!(arguments.len(), 1); - assert_eq!(results.len(), 2); - let old = arguments[0]; - let new = results[1]; - assert!(matches!(dfg.type_of_value(old), Type::Array(_, _))); - assert!(matches!(dfg.type_of_value(new), Type::Slice(_))); - SizeChange::SetTo { old, new } - } - - // These cases don't affect slice capacities - Intrinsic::AssertConstant - | Intrinsic::StaticAssert - | Intrinsic::ApplyRangeConstraint - | Intrinsic::ArrayLen - | Intrinsic::ArrayAsStrUnchecked - | Intrinsic::StrAsBytes - | Intrinsic::BlackBox(_) - | Intrinsic::Hint(Hint::BlackBox) - | Intrinsic::AsWitness - | Intrinsic::IsUnconstrained - | Intrinsic::DerivePedersenGenerators - | Intrinsic::ToBits(_) - | Intrinsic::ToRadix(_) - | Intrinsic::ArrayRefCount - | Intrinsic::SliceRefCount - | Intrinsic::FieldLessThan => SizeChange::None, - } -} - #[cfg(debug_assertions)] fn remove_if_else_pre_check(func: &Function) { // This pass should only run post-flattening. diff --git a/test_programs/compile_success_no_bug/regression_10136/Nargo.toml b/test_programs/compile_success_no_bug/regression_10136/Nargo.toml new file mode 100644 index 00000000000..728c24db7f2 --- /dev/null +++ b/test_programs/compile_success_no_bug/regression_10136/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_10136" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/regression_10136/src/main.nr b/test_programs/compile_success_no_bug/regression_10136/src/main.nr new file mode 100644 index 00000000000..334b7d910e3 --- /dev/null +++ b/test_programs/compile_success_no_bug/regression_10136/src/main.nr @@ -0,0 +1,8 @@ +fn main(array: [Field; 1], cond: bool) -> pub u32 { + let slice = array.as_slice(); + let mut slice = std::hint::black_box(slice); + if cond { + slice = slice.push_back(1); + } + slice.len() +} diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/regression_10136/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/regression_10136/execute__tests__expanded.snap new file mode 100644 index 00000000000..49acfc479ec --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/regression_10136/execute__tests__expanded.snap @@ -0,0 +1,10 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main(array: [Field; 1], cond: bool) -> pub u32 { + let slice: [Field] = array.as_slice(); + let mut slice: [Field] = std::hint::black_box(slice); + if cond { slice = slice.push_back(1_Field); }; + slice.len() +}