From 45eb7568d56b2d254453b85f236d554232aa5df9 Mon Sep 17 00:00:00 2001 From: jfecher Date: Fri, 22 Nov 2024 14:12:25 -0600 Subject: [PATCH] feat: Add `array_refcount` and `slice_refcount` builtins for debugging (#6584) --- compiler/noirc_evaluator/src/acir/mod.rs | 7 + .../src/brillig/brillig_gen/brillig_block.rs | 427 ++++++++++-------- .../check_for_underconstrained_values.rs | 4 +- .../noirc_evaluator/src/ssa/ir/instruction.rs | 10 + .../src/ssa/ir/instruction/call.rs | 2 + .../src/ssa/opt/remove_enable_side_effects.rs | 2 + .../src/ssa/opt/remove_if_else.rs | 2 + .../src/hir/comptime/interpreter/builtin.rs | 2 + cspell.json | 1 + docs/docs/noir/standard_library/mem.md | 30 ++ noir_stdlib/src/mem.nr | 14 + .../reference_counts/Nargo.toml | 7 + .../reference_counts/Prover.toml | 2 + .../reference_counts/src/main.nr | 40 ++ tooling/debugger/ignored-tests.txt | 3 +- tooling/nargo_cli/build.rs | 6 +- 16 files changed, 363 insertions(+), 196 deletions(-) create mode 100644 test_programs/execution_success/reference_counts/Nargo.toml create mode 100644 test_programs/execution_success/reference_counts/Prover.toml create mode 100644 test_programs/execution_success/reference_counts/src/main.nr diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 46d0924b322..7274fe908d1 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -2762,6 +2762,13 @@ impl<'a> Context<'a> { Intrinsic::FieldLessThan => { unreachable!("FieldLessThan can only be called in unconstrained") } + Intrinsic::ArrayRefCount | Intrinsic::SliceRefCount => { + let zero = self.acir_context.add_constant(FieldElement::zero()); + Ok(vec![AcirValue::Var( + zero, + AcirType::NumericType(NumericType::Unsigned { bit_size: 32 }), + )]) + } } } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 36e1ee90e11..1fa4985295a 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -402,210 +402,251 @@ impl<'block> BrilligBlock<'block> { let result_ids = dfg.instruction_results(instruction_id); self.convert_ssa_function_call(*func_id, arguments, dfg, result_ids); } - Value::Intrinsic(Intrinsic::BlackBox(bb_func)) => { - // Slices are represented as a tuple of (length, slice contents). - // We must check the inputs to determine if there are slices - // and make sure that we pass the correct inputs to the black box function call. - // The loop below only keeps the slice contents, so that - // setting up a black box function with slice inputs matches the expected - // number of arguments specified in the function signature. - let mut arguments_no_slice_len = Vec::new(); - for (i, arg) in arguments.iter().enumerate() { - if matches!(dfg.type_of_value(*arg), Type::Numeric(_)) { - if i < arguments.len() - 1 { - if !matches!(dfg.type_of_value(arguments[i + 1]), Type::Slice(_)) { - arguments_no_slice_len.push(*arg); - } + Value::Intrinsic(intrinsic) => { + // This match could be combined with the above but without it rust analyzer + // can't automatically insert any missing cases + match intrinsic { + Intrinsic::ArrayLen => { + let result_variable = self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + dfg.instruction_results(instruction_id)[0], + dfg, + ); + let param_id = arguments[0]; + // Slices are represented as a tuple in the form: (length, slice contents). + // Thus, we can expect the first argument to a field in the case of a slice + // or an array in the case of an array. + if let Type::Numeric(_) = dfg.type_of_value(param_id) { + let len_variable = self.convert_ssa_value(arguments[0], dfg); + let length = len_variable.extract_single_addr(); + self.brillig_context + .mov_instruction(result_variable.address, length.address); } else { - arguments_no_slice_len.push(*arg); + self.convert_ssa_array_len( + arguments[0], + result_variable.address, + dfg, + ); } - } else { - arguments_no_slice_len.push(*arg); } - } - - let function_arguments = - vecmap(&arguments_no_slice_len, |arg| self.convert_ssa_value(*arg, dfg)); - let function_results = dfg.instruction_results(instruction_id); - let function_results = vecmap(function_results, |result| { - self.allocate_external_call_result(*result, dfg) - }); - convert_black_box_call( - self.brillig_context, - bb_func, - &function_arguments, - &function_results, - ); - } - Value::Intrinsic(Intrinsic::ArrayLen) => { - let result_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - dfg.instruction_results(instruction_id)[0], - dfg, - ); - let param_id = arguments[0]; - // Slices are represented as a tuple in the form: (length, slice contents). - // Thus, we can expect the first argument to a field in the case of a slice - // or an array in the case of an array. - if let Type::Numeric(_) = dfg.type_of_value(param_id) { - let len_variable = self.convert_ssa_value(arguments[0], dfg); - let length = len_variable.extract_single_addr(); - self.brillig_context - .mov_instruction(result_variable.address, length.address); - } else { - self.convert_ssa_array_len(arguments[0], result_variable.address, dfg); - } - } - Value::Intrinsic(Intrinsic::AsSlice) => { - let source_variable = self.convert_ssa_value(arguments[0], dfg); - let result_ids = dfg.instruction_results(instruction_id); - let destination_len_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - result_ids[0], - dfg, - ); - let destination_variable = self.variables.define_variable( - self.function_context, - self.brillig_context, - result_ids[1], - dfg, - ); - let destination_vector = destination_variable.extract_vector(); - let source_array = source_variable.extract_array(); - let element_size = dfg.type_of_value(arguments[0]).element_size(); - - let source_size_register = self - .brillig_context - .make_usize_constant_instruction(source_array.size.into()); - - // we need to explicitly set the destination_len_variable - self.brillig_context.codegen_usize_op( - source_size_register.address, - destination_len_variable.address, - BrilligBinaryOp::UnsignedDiv, - element_size, - ); - - self.brillig_context.codegen_initialize_vector( - destination_vector, - source_size_register, - None, - ); - - // Items - let vector_items_pointer = - self.brillig_context.codegen_make_vector_items_pointer(destination_vector); - let array_items_pointer = - self.brillig_context.codegen_make_array_items_pointer(source_array); - - self.brillig_context.codegen_mem_copy( - array_items_pointer, - vector_items_pointer, - source_size_register, - ); - - self.brillig_context.deallocate_single_addr(source_size_register); - self.brillig_context.deallocate_register(vector_items_pointer); - self.brillig_context.deallocate_register(array_items_pointer); - } - Value::Intrinsic( - Intrinsic::SlicePushBack - | Intrinsic::SlicePopBack - | Intrinsic::SlicePushFront - | Intrinsic::SlicePopFront - | Intrinsic::SliceInsert - | Intrinsic::SliceRemove, - ) => { - self.convert_ssa_slice_intrinsic_call( - dfg, - &dfg[dfg.resolve(*func)], - instruction_id, - arguments, - ); - } - Value::Intrinsic(Intrinsic::ToRadix(endianness)) => { - let results = dfg.instruction_results(instruction_id); - - let source = self.convert_ssa_single_addr_value(arguments[0], dfg); - let radix = self.convert_ssa_single_addr_value(arguments[1], dfg); - - let target_array = self - .variables - .define_variable( - self.function_context, - self.brillig_context, - results[0], - dfg, - ) - .extract_array(); - - self.brillig_context.codegen_to_radix( - source, - target_array, - radix, - matches!(endianness, Endian::Little), - false, - ); - } - Value::Intrinsic(Intrinsic::ToBits(endianness)) => { - let results = dfg.instruction_results(instruction_id); + Intrinsic::AsSlice => { + let source_variable = self.convert_ssa_value(arguments[0], dfg); + let result_ids = dfg.instruction_results(instruction_id); + let destination_len_variable = + self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + result_ids[0], + dfg, + ); + let destination_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + result_ids[1], + dfg, + ); + let destination_vector = destination_variable.extract_vector(); + let source_array = source_variable.extract_array(); + let element_size = dfg.type_of_value(arguments[0]).element_size(); - let source = self.convert_ssa_single_addr_value(arguments[0], dfg); + let source_size_register = self + .brillig_context + .make_usize_constant_instruction(source_array.size.into()); + + // we need to explicitly set the destination_len_variable + self.brillig_context.codegen_usize_op( + source_size_register.address, + destination_len_variable.address, + BrilligBinaryOp::UnsignedDiv, + element_size, + ); - let target_array = self - .variables - .define_variable( - self.function_context, - self.brillig_context, - results[0], - dfg, - ) - .extract_array(); + self.brillig_context.codegen_initialize_vector( + destination_vector, + source_size_register, + None, + ); - let two = self.brillig_context.make_usize_constant_instruction(2_usize.into()); + // Items + let vector_items_pointer = self + .brillig_context + .codegen_make_vector_items_pointer(destination_vector); + let array_items_pointer = + self.brillig_context.codegen_make_array_items_pointer(source_array); + + self.brillig_context.codegen_mem_copy( + array_items_pointer, + vector_items_pointer, + source_size_register, + ); - self.brillig_context.codegen_to_radix( - source, - target_array, - two, - matches!(endianness, Endian::Little), - true, - ); + self.brillig_context.deallocate_single_addr(source_size_register); + self.brillig_context.deallocate_register(vector_items_pointer); + self.brillig_context.deallocate_register(array_items_pointer); + } + Intrinsic::SlicePushBack + | Intrinsic::SlicePopBack + | Intrinsic::SlicePushFront + | Intrinsic::SlicePopFront + | Intrinsic::SliceInsert + | Intrinsic::SliceRemove => { + self.convert_ssa_slice_intrinsic_call( + dfg, + &dfg[dfg.resolve(*func)], + instruction_id, + arguments, + ); + } + Intrinsic::ToBits(endianness) => { + let results = dfg.instruction_results(instruction_id); + + let source = self.convert_ssa_single_addr_value(arguments[0], dfg); + + let target_array = self + .variables + .define_variable( + self.function_context, + self.brillig_context, + results[0], + dfg, + ) + .extract_array(); + + let two = self + .brillig_context + .make_usize_constant_instruction(2_usize.into()); + + self.brillig_context.codegen_to_radix( + source, + target_array, + two, + matches!(endianness, Endian::Little), + true, + ); - self.brillig_context.deallocate_single_addr(two); - } + self.brillig_context.deallocate_single_addr(two); + } - // `Intrinsic::AsWitness` is used to provide hints to acir-gen on optimal expression splitting. - // It is then useless in the brillig runtime and so we can ignore it - Value::Intrinsic(Intrinsic::AsWitness) => (), - Value::Intrinsic(Intrinsic::FieldLessThan) => { - let lhs = self.convert_ssa_single_addr_value(arguments[0], dfg); - assert!(lhs.bit_size == FieldElement::max_num_bits()); - let rhs = self.convert_ssa_single_addr_value(arguments[1], dfg); - assert!(rhs.bit_size == FieldElement::max_num_bits()); - - let results = dfg.instruction_results(instruction_id); - let destination = self - .variables - .define_variable( - self.function_context, - self.brillig_context, - results[0], - dfg, - ) - .extract_single_addr(); - assert!(destination.bit_size == 1); + Intrinsic::ToRadix(endianness) => { + let results = dfg.instruction_results(instruction_id); + + let source = self.convert_ssa_single_addr_value(arguments[0], dfg); + let radix = self.convert_ssa_single_addr_value(arguments[1], dfg); + + let target_array = self + .variables + .define_variable( + self.function_context, + self.brillig_context, + results[0], + dfg, + ) + .extract_array(); + + self.brillig_context.codegen_to_radix( + source, + target_array, + radix, + matches!(endianness, Endian::Little), + false, + ); + } + Intrinsic::BlackBox(bb_func) => { + // Slices are represented as a tuple of (length, slice contents). + // We must check the inputs to determine if there are slices + // and make sure that we pass the correct inputs to the black box function call. + // The loop below only keeps the slice contents, so that + // setting up a black box function with slice inputs matches the expected + // number of arguments specified in the function signature. + let mut arguments_no_slice_len = Vec::new(); + for (i, arg) in arguments.iter().enumerate() { + if matches!(dfg.type_of_value(*arg), Type::Numeric(_)) { + if i < arguments.len() - 1 { + if !matches!( + dfg.type_of_value(arguments[i + 1]), + Type::Slice(_) + ) { + arguments_no_slice_len.push(*arg); + } + } else { + arguments_no_slice_len.push(*arg); + } + } else { + arguments_no_slice_len.push(*arg); + } + } - self.brillig_context.binary_instruction( - lhs, - rhs, - destination, - BrilligBinaryOp::LessThan, - ); + let function_arguments = vecmap(&arguments_no_slice_len, |arg| { + self.convert_ssa_value(*arg, dfg) + }); + let function_results = dfg.instruction_results(instruction_id); + let function_results = vecmap(function_results, |result| { + self.allocate_external_call_result(*result, dfg) + }); + convert_black_box_call( + self.brillig_context, + bb_func, + &function_arguments, + &function_results, + ); + } + // `Intrinsic::AsWitness` is used to provide hints to acir-gen on optimal expression splitting. + // It is then useless in the brillig runtime and so we can ignore it + Intrinsic::AsWitness => (), + Intrinsic::FieldLessThan => { + let lhs = self.convert_ssa_single_addr_value(arguments[0], dfg); + assert!(lhs.bit_size == FieldElement::max_num_bits()); + let rhs = self.convert_ssa_single_addr_value(arguments[1], dfg); + assert!(rhs.bit_size == FieldElement::max_num_bits()); + + let results = dfg.instruction_results(instruction_id); + let destination = self + .variables + .define_variable( + self.function_context, + self.brillig_context, + results[0], + dfg, + ) + .extract_single_addr(); + assert!(destination.bit_size == 1); + + self.brillig_context.binary_instruction( + lhs, + rhs, + destination, + BrilligBinaryOp::LessThan, + ); + } + Intrinsic::ArrayRefCount | Intrinsic::SliceRefCount => { + let array = self.convert_ssa_value(arguments[0], dfg); + let result = dfg.instruction_results(instruction_id)[0]; + + let destination = self.variables.define_variable( + self.function_context, + self.brillig_context, + result, + dfg, + ); + let destination = destination.extract_register(); + let array = array.extract_register(); + self.brillig_context.load_instruction(destination, array); + } + Intrinsic::FromField + | Intrinsic::AsField + | Intrinsic::IsUnconstrained + | Intrinsic::DerivePedersenGenerators + | Intrinsic::ApplyRangeConstraint + | Intrinsic::StrAsBytes + | Intrinsic::AssertConstant + | Intrinsic::StaticAssert + | Intrinsic::ArrayAsStrUnchecked => { + unreachable!("unsupported function call type {:?}", dfg[*func]) + } + } } - _ => { + Value::Instruction { .. } | Value::Param { .. } | Value::NumericConstant { .. } => { unreachable!("unsupported function call type {:?}", dfg[*func]) } }, diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index cf884c98be9..7a4e336c33e 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -205,16 +205,18 @@ impl Context { | Intrinsic::IsUnconstrained => {} Intrinsic::ArrayLen | Intrinsic::ArrayAsStrUnchecked + | Intrinsic::ArrayRefCount | Intrinsic::AsField | Intrinsic::AsSlice | Intrinsic::BlackBox(..) | Intrinsic::DerivePedersenGenerators | Intrinsic::FromField + | Intrinsic::SliceInsert | Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SlicePopBack | Intrinsic::SlicePopFront - | Intrinsic::SliceInsert + | Intrinsic::SliceRefCount | Intrinsic::SliceRemove | Intrinsic::StaticAssert | Intrinsic::StrAsBytes diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 3ac0b2e3f5e..9958aadd443 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -71,6 +71,8 @@ pub(crate) enum Intrinsic { IsUnconstrained, DerivePedersenGenerators, FieldLessThan, + ArrayRefCount, + SliceRefCount, } impl std::fmt::Display for Intrinsic { @@ -100,6 +102,8 @@ impl std::fmt::Display for Intrinsic { Intrinsic::IsUnconstrained => write!(f, "is_unconstrained"), Intrinsic::DerivePedersenGenerators => write!(f, "derive_pedersen_generators"), Intrinsic::FieldLessThan => write!(f, "field_less_than"), + Intrinsic::ArrayRefCount => write!(f, "array_refcount"), + Intrinsic::SliceRefCount => write!(f, "slice_refcount"), } } } @@ -113,6 +117,10 @@ impl Intrinsic { Intrinsic::AssertConstant | Intrinsic::StaticAssert | Intrinsic::ApplyRangeConstraint + // Array & slice ref counts are treated as having side effects since they operate + // on hidden variables on otherwise identical array values. + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount | Intrinsic::AsWitness => true, // These apply a constraint that the input must fit into a specified number of limbs. @@ -171,6 +179,8 @@ impl Intrinsic { "is_unconstrained" => Some(Intrinsic::IsUnconstrained), "derive_pedersen_generators" => Some(Intrinsic::DerivePedersenGenerators), "field_less_than" => Some(Intrinsic::FieldLessThan), + "array_refcount" => Some(Intrinsic::ArrayRefCount), + "slice_refcount" => Some(Intrinsic::SliceRefCount), other => BlackBoxFunc::lookup(other).map(Intrinsic::BlackBox), } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 7e41512fd8f..4be37b3c626 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -368,6 +368,8 @@ pub(super) fn simplify_call( SimplifyResult::None } } + Intrinsic::ArrayRefCount => SimplifyResult::None, + Intrinsic::SliceRefCount => SimplifyResult::None, } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index 0517f9ef89f..f735d9300ce 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -180,6 +180,8 @@ impl Context { | Intrinsic::AsWitness | Intrinsic::IsUnconstrained | Intrinsic::DerivePedersenGenerators + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount | Intrinsic::FieldLessThan => false, }, 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 8076bc3cc99..8e25c3f0a35 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -232,6 +232,8 @@ fn slice_capacity_change( | Intrinsic::DerivePedersenGenerators | Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount | Intrinsic::FieldLessThan => SizeChange::None, } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 80c1ee217c2..e8b34ab4323 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -60,6 +60,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "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), + "array_refcount" => Ok(Value::U32(0)), "assert_constant" => Ok(Value::Bool(true)), "as_slice" => as_slice(interner, arguments, location), "ctstring_eq" => ctstring_eq(arguments, location), @@ -167,6 +168,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "slice_pop_front" => slice_pop_front(interner, arguments, location, call_stack), "slice_push_back" => slice_push_back(interner, arguments, location), "slice_push_front" => slice_push_front(interner, arguments, location), + "slice_refcount" => Ok(Value::U32(0)), "slice_remove" => slice_remove(interner, arguments, location, call_stack), "str_as_bytes" => str_as_bytes(interner, arguments, location), "str_as_ctstring" => str_as_ctstring(interner, arguments, location), diff --git a/cspell.json b/cspell.json index b0cd635e46f..36bba737cd7 100644 --- a/cspell.json +++ b/cspell.json @@ -183,6 +183,7 @@ "quantile", "quasiquote", "rangemap", + "refcount", "repr", "reqwest", "rfind", diff --git a/docs/docs/noir/standard_library/mem.md b/docs/docs/noir/standard_library/mem.md index 95d36ac2a72..3619550273e 100644 --- a/docs/docs/noir/standard_library/mem.md +++ b/docs/docs/noir/standard_library/mem.md @@ -50,3 +50,33 @@ by checking this equality once `N`, `A`, `B` are fully resolved. Note that since this safety check is performed after type checking rather than during, no error is issued if the function containing `checked_transmute` is never called. + +# `std::mem::array_refcount` + +```rust +fn array_refcount(array: [T; N]) -> u32 {} +``` + +Returns the internal reference count of an array value in unconstrained code. + +Arrays only have reference count in unconstrained code - using this anywhere +else will return zero. + +This function is mostly intended for debugging compiler optimizations but can also be used +to find where array copies may be happening in unconstrained code by placing it before array +mutations. + +# `std::mem::slice_refcount` + +```rust +fn slice_refcount(slice: [T]) -> u32 {} +``` + +Returns the internal reference count of a slice value in unconstrained code. + +Slices only have reference count in unconstrained code - using this anywhere +else will return zero. + +This function is mostly intended for debugging compiler optimizations but can also be used +to find where slice copies may be happening in unconstrained code by placing it before slice +mutations. diff --git a/noir_stdlib/src/mem.nr b/noir_stdlib/src/mem.nr index 0d47a21b50d..23125867eab 100644 --- a/noir_stdlib/src/mem.nr +++ b/noir_stdlib/src/mem.nr @@ -15,3 +15,17 @@ pub fn zeroed() -> T {} /// that it is equal to the previous. #[builtin(checked_transmute)] pub fn checked_transmute(value: T) -> U {} + +/// Returns the internal reference count of an array value in unconstrained code. +/// +/// Arrays only have reference count in unconstrained code - using this anywhere +/// else will return zero. +#[builtin(array_refcount)] +pub fn array_refcount(array: [T; N]) -> u32 {} + +/// Returns the internal reference count of a slice value in unconstrained code. +/// +/// Slices only have reference count in unconstrained code - using this anywhere +/// else will return zero. +#[builtin(slice_refcount)] +pub fn slice_refcount(slice: [T]) -> u32 {} diff --git a/test_programs/execution_success/reference_counts/Nargo.toml b/test_programs/execution_success/reference_counts/Nargo.toml new file mode 100644 index 00000000000..ae787e0ccb9 --- /dev/null +++ b/test_programs/execution_success/reference_counts/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "reference_counts" +type = "bin" +authors = [""] +compiler_version = ">=0.35.0" + +[dependencies] diff --git a/test_programs/execution_success/reference_counts/Prover.toml b/test_programs/execution_success/reference_counts/Prover.toml new file mode 100644 index 00000000000..c01dd9462d8 --- /dev/null +++ b/test_programs/execution_success/reference_counts/Prover.toml @@ -0,0 +1,2 @@ +x = 5 +b = true diff --git a/test_programs/execution_success/reference_counts/src/main.nr b/test_programs/execution_success/reference_counts/src/main.nr new file mode 100644 index 00000000000..7ab7de893fa --- /dev/null +++ b/test_programs/execution_success/reference_counts/src/main.nr @@ -0,0 +1,40 @@ +fn main() { + let mut array = [0, 1, 2]; + assert_refcount(array, 1); + + borrow(array, std::mem::array_refcount(array)); + borrow_mut(&mut array, std::mem::array_refcount(array)); + copy_mut(array, std::mem::array_refcount(array)); +} + +fn borrow(array: [Field; 3], rc_before_call: u32) { + assert_refcount(array, rc_before_call); + println(array[0]); +} + +fn borrow_mut(array: &mut [Field; 3], rc_before_call: u32) { + assert_refcount(*array, rc_before_call + 0); // Issue! This should be rc_before_call + 1 + array[0] = 5; + println(array[0]); +} + +fn copy_mut(mut array: [Field; 3], rc_before_call: u32) { + assert_refcount(array, rc_before_call + 0); // Issue! This should be rc_before_call + 1 + array[0] = 6; + println(array[0]); +} + +fn assert_refcount(array: [Field; 3], expected: u32) { + let count = std::mem::array_refcount(array); + + // All refcounts are zero when running this as a constrained program + if std::runtime::is_unconstrained() { + if count != expected { + // Brillig doesn't print the actual & expected arguments on assertion failure + println(f"actual = {count}, expected = {expected}"); + } + assert_eq(count, expected); + } else { + assert_eq(count, 0); + } +} diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index 0037b8e5d5f..e0548fe1e1a 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -2,7 +2,8 @@ brillig_references debug_logs is_unconstrained macros +reference_counts references regression_4709 reference_only_used_as_alias -brillig_rc_regression_6123 \ No newline at end of file +brillig_rc_regression_6123 diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index ad1f82f4e45..04eb0ea0f52 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -60,9 +60,13 @@ const IGNORED_BRILLIG_TESTS: [&str; 11] = [ ]; /// Tests which aren't expected to work with the default inliner cases. -const INLINER_MIN_OVERRIDES: [(&str, i64); 1] = [ +const INLINER_MIN_OVERRIDES: [(&str, i64); 2] = [ // 0 works if PoseidonHasher::write is tagged as `inline_always`, otherwise 22. ("eddsa", 0), + // (#6583): The RcTracker in the DIE SSA pass is removing inc_rcs that are still needed. + // This triggers differently depending on the optimization level (although all are wrong), + // so we arbitrarily only run with the inlined versions. + ("reference_counts", 0), ]; /// Some tests are expected to have warnings