diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs index 3d76c464385..50b386c1734 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs @@ -117,7 +117,7 @@ pub(crate) fn simplify( let array_or_slice_type = dfg.type_of_value(*array); if matches!(array_or_slice_type, Type::Array(_, 1)) - && array_or_slice_type.flattened_size() == 1 + && array_or_slice_type.element_size() == 1 { // If the array is of length 1 then we know the only value which can be potentially read out of it. // We can then simply assert that the index is equal to zero and return the array's contained value. @@ -336,6 +336,11 @@ pub(crate) fn simplify( /// v4 = array_get v2, index u32 0 -> Field /// ``` /// We then attempt to resolve the array read immediately. +/// +/// Note that this does not work if the array has length 1, but contains a complex type such as tuple, +/// which consists of multiple elements. If that is the case than the `index` will most likely not be +/// a constant, but a base plus an offset, and if the array contains repeated elements of the same type +/// for example, we wouldn't be able to come up with a constant offset even if we knew the return type. fn optimize_length_one_array_read( dfg: &mut DataFlowGraph, block: BasicBlockId, @@ -365,7 +370,7 @@ fn optimize_length_one_array_read( /// v3 = array_set v2, index 2, value: 7 /// v4 = array_get v3, index 1 /// -/// We want to optimize `v4` to `10`. To do this we need to follow the array value +/// We want to optimize `v4` to `11`. To do this we need to follow the array value /// through several array sets. For each array set: /// - If the index is non-constant we fail the optimization since any index may be changed /// - If the index is constant and is our target index, we conservatively fail the optimization @@ -374,6 +379,10 @@ fn optimize_length_one_array_read( /// - Otherwise, we check the array value of the array set. /// - If the array value is constant, we use that array. /// - If the array value is from a previous array-set, we recur. +/// +/// That is, we have multiple `array_set` instructions setting various constant indexes +/// of the same array, returning a modified version. We want to go backwards until we +/// find the last `array_set` for the index we are interested in, and return the value set. fn try_optimize_array_get_from_previous_set( dfg: &DataFlowGraph, mut array_id: ValueId, @@ -646,6 +655,41 @@ mod tests { "#); } + #[test] + fn does_not_use_flattened_size_for_length_one_array_check() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field, v1: u32): + v2 = make_array [v0, v0] : [Field; 2] + v3 = make_array [v2] : [[Field; 2]; 1] + v4 = make_array [] : [Field; 0] + v5 = make_array [v4, v0] : [([Field; 0], Field); 1] + v6 = array_get v3, index v1 -> [Field; 2] + v7 = add v1, u32 1 + v8 = array_get v5, index v7 -> Field + return v6, v8 + } + "; + // The flattened size of v3 is 2, but it has 1 element -> it can be optimized. + // The flattened size of v5 is 1, but it has 2 elements -> it cannot be optimized. + + let ssa = Ssa::from_str_simplifying(src).unwrap(); + + assert_ssa_snapshot!(ssa, @r#" + acir(inline) fn main f0 { + b0(v0: Field, v1: u32): + v2 = make_array [v0, v0] : [Field; 2] + v3 = make_array [v2] : [[Field; 2]; 1] + v4 = make_array [] : [Field; 0] + v5 = make_array [v4, v0] : [([Field; 0], Field); 1] + constrain v1 == u32 0, "Index out of bounds" + v8 = add v1, u32 1 + v9 = array_get v5, index v8 -> Field + return v2, v9 + } + "#); + } + #[test] fn does_not_crash_on_truncated_division_with_large_denominators() { // There can be invalid division instructions which have extremely large denominators diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 86e779c1e87..a964ca5103a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -253,7 +253,11 @@ impl Type { } } - /// Returns the flattened size of a Type + /// Returns the flattened size of a Type. + /// + /// The flattened type is mostly useful in ACIR, where nested arrays are also flattened, + /// as opposed to SSA, where only tuples get flattened into the array they are in, + /// but nested arrays appear as a value ID. pub(crate) fn flattened_size(&self) -> u32 { match self { Type::Array(elements, len) => { diff --git a/test_programs/execution_panic/regression_9986/Nargo.toml b/test_programs/execution_failure/regression_9986/Nargo.toml similarity index 100% rename from test_programs/execution_panic/regression_9986/Nargo.toml rename to test_programs/execution_failure/regression_9986/Nargo.toml diff --git a/test_programs/execution_panic/regression_9986/src/main.nr b/test_programs/execution_failure/regression_9986/src/main.nr similarity index 100% rename from test_programs/execution_panic/regression_9986/src/main.nr rename to test_programs/execution_failure/regression_9986/src/main.nr diff --git a/test_programs/execution_success/regression_10141/Nargo.toml b/test_programs/execution_success/regression_10141/Nargo.toml new file mode 100644 index 00000000000..84a5e6d66ae --- /dev/null +++ b/test_programs/execution_success/regression_10141/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_10141" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/regression_10141/Prover.toml b/test_programs/execution_success/regression_10141/Prover.toml new file mode 100644 index 00000000000..38025d3c3ad --- /dev/null +++ b/test_programs/execution_success/regression_10141/Prover.toml @@ -0,0 +1,2 @@ +a = 0 +return = 10 diff --git a/test_programs/execution_success/regression_10141/src/main.nr b/test_programs/execution_success/regression_10141/src/main.nr new file mode 100644 index 00000000000..83d2fe5236c --- /dev/null +++ b/test_programs/execution_success/regression_10141/src/main.nr @@ -0,0 +1,7 @@ +fn main(a: u32) -> pub Field { + foo()[a].1 as Field +} + +fn foo() -> [([u64; 0], u16); 1] { + [([], 10)] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_10141/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_10141/execute__tests__expanded.snap new file mode 100644 index 00000000000..92a41e69772 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_10141/execute__tests__expanded.snap @@ -0,0 +1,11 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main(a: u32) -> pub Field { + foo()[a].1 as Field +} + +fn foo() -> [([u64; 0], u16); 1] { + [([], 10_u16)] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_10141/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_10141/execute__tests__stdout.snap new file mode 100644 index 00000000000..30897d6fd9b --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_10141/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- +[regression_10141] Circuit output: 0x0a