diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 005fbc481cc..5be2e71f856 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -1,4 +1,4 @@ -//! The loop invariant code motion pass moves code from inside a loop to before the loop +//! The loop invariant code motion (LICM) pass moves code from inside a loop to before the loop //! if that code will always have the same result on every iteration of the loop. //! //! To identify a loop invariant, check whether all of an instruction's values are: @@ -549,7 +549,7 @@ impl<'f> LoopInvariantContext<'f> { /// We know a loop body will execute if we have constant loop bounds where the upper bound /// is greater than the lower bound. fn does_loop_body_execute(&self) -> bool { - // The loop will never be executed if we have an upper bound of zero, equal loop bounds, + // The loop will never be executed if we have equal loop bounds // or we are unsure if the loop will ever be executed (dynamic loop bounds). // If certain instructions were to be hoisted out of a loop that never executed it // could potentially cause the program to fail when it is not meant to fail. @@ -699,7 +699,7 @@ impl<'f> LoopInvariantContext<'f> { .map(|(is_left, min, max)| (is_left, min, max, binary)) } - /// If the inputs are an induction and a loop invariant variables, it returns + /// If the inputs are an induction and loop invariant variables, it returns /// the maximum and minimum values of the induction variable, based on the loop bounds, /// and a boolean indicating if the induction variable is on the lhs or rhs (true for lhs) fn match_induction_and_invariant( @@ -716,7 +716,7 @@ impl<'f> LoopInvariantContext<'f> { _ => None, }?; - assert!(!upper.is_zero(), "executing a non executable loop"); + assert!(self.does_loop_body_execute(), "executing a non executable loop"); let (upper_field, upper_type) = upper.dec().into_numeric_constant(); let (lower_field, lower_type) = lower.into_numeric_constant(); @@ -1665,6 +1665,40 @@ mod test { assert_normalized_ssa_equals(ssa, expected); } + + #[test] + fn negative_lower_bound() { + // Regression fro issue #8858 (https://github.com/noir-lang/noir/issues/8858) that we + // do not panic on a negative lower bound + let src = " + acir(inline) predicate_pure fn main f0 { + b0(): + jmp b1(i32 4294967295) + b1(v0: i32): + v3 = lt v0, i32 0 + jmpif v3 then: b2, else: b3 + b2(): + v4 = truncate v0 to 32 bits, max_bit_size: 33 + v5 = cast v4 as u32 + v6 = cast v0 as u32 + v8 = lt v6, u32 2147483648 + v9 = lt v5, u32 2147483648 + v10 = eq v9, v8 + v11 = unchecked_mul v10, v8 + constrain v11 == v8 + v12 = lt v0, v4 + constrain v12 == u1 0 + v15 = unchecked_add v0, i32 1 + jmp b1(v15) + b3(): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.loop_invariant_code_motion(); + assert_normalized_ssa_equals(ssa, src); + } } #[cfg(test)] diff --git a/cspell.json b/cspell.json index 4167a516ca6..ad5b1f3c159 100644 --- a/cspell.json +++ b/cspell.json @@ -155,6 +155,7 @@ "kosaraju", "krate", "libc", + "LICM", "Linea", "lookback", "losslessly", diff --git a/test_programs/compile_success_empty/signed_lower_loop_bound_regression_8858/Nargo.toml b/test_programs/compile_success_empty/signed_lower_loop_bound_regression_8858/Nargo.toml new file mode 100644 index 00000000000..ba6f67ef60e --- /dev/null +++ b/test_programs/compile_success_empty/signed_lower_loop_bound_regression_8858/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "signed_lower_loop_bound_regression_8858" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/signed_lower_loop_bound_regression_8858/src/main.nr b/test_programs/compile_success_empty/signed_lower_loop_bound_regression_8858/src/main.nr new file mode 100644 index 00000000000..e5518ecdb1e --- /dev/null +++ b/test_programs/compile_success_empty/signed_lower_loop_bound_regression_8858/src/main.nr @@ -0,0 +1,6 @@ +fn main() { + let lower: i32 = -1; + for i in lower..0 { + assert(i >= (i + 0)); + } +} diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_empty/signed_lower_loop_bound_regression_8858/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_empty/signed_lower_loop_bound_regression_8858/execute__tests__expanded.snap new file mode 100644 index 00000000000..1ea29632cac --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_empty/signed_lower_loop_bound_regression_8858/execute__tests__expanded.snap @@ -0,0 +1,10 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() { + let lower: i32 = -1; + for i in lower..0 { + assert(i >= (i + 0)); + } +} diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_empty/signed_lower_loop_bound_regression_8858/execute__tests__force_brillig_false_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/compile_success_empty/signed_lower_loop_bound_regression_8858/execute__tests__force_brillig_false_inliner_0.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_empty/signed_lower_loop_bound_regression_8858/execute__tests__force_brillig_false_inliner_0.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +}