fix: Use IntegerConstant for loop boundaries in unrolling

compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

@@ -202,7 +202,10 @@
 /// Values in the range `[0, 2^(bit_size-1))` are interpreted as positive integers
 ///
 /// Values in the range `[2^(bit_size-1), 2^bit_size)` are interpreted as negative integers.
-fn try_convert_field_element_to_signed_integer(field: FieldElement, bit_size: u32) -> Option<i128> {
+pub(crate) fn try_convert_field_element_to_signed_integer(
+    field: FieldElement,
+    bit_size: u32,
+) -> Option<i128> {
     let unsigned_int = truncate(field.try_into_u128()?, bit_size);
 
     let max_positive_value = 1 << (bit_size - 1);
@@ -219,7 +222,7 @@
     Some(signed_int)
 }
 
-fn convert_signed_integer_to_field_element(int: i128, bit_size: u32) -> FieldElement {
+pub(crate) fn convert_signed_integer_to_field_element(int: i128, bit_size: u32) -> FieldElement {
     if int >= 0 {
         FieldElement::from(int)
     } else {
@@ -358,7 +361,7 @@
 
             let integer_modulus = BigUint::from(2_u128).pow(bit_size);
             let truncated_as_bigint = BigUint::from(input)
                         .modpow(&BigUint::one(), &integer_modulus);
             let truncated_as_bigint = FieldElement::from_be_bytes_reduce(&truncated_as_bigint.to_bytes_be());
             prop_assert_eq!(truncated_as_field, truncated_as_bigint);
         }
@@ -375,4 +378,14 @@
         assert!(BinaryOp::Shr.get_i128_function()(1, 128).is_none());
         assert!(BinaryOp::Shl.get_i128_function()(1, 128).is_none());
     }
+
+    #[test]
+    fn test_plus_minus_one_as_field() {
+        for (i, u) in [(-1i64, u64::MAX), (-2i64, u64::MAX - 1), (1i64, 1u64)] {
+            let i: i128 = i.into();
+            let f = convert_signed_integer_to_field_element(i, 64);
+            assert_eq!(f.to_u128(), u as u128);
+            assert_eq!(i, try_convert_field_element_to_signed_integer(f, 64).unwrap());
+        }
+    }
 }

compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

@@ -59,7 +59,7 @@ use crate::ssa::{
         function_inserter::FunctionInserter,
         instruction::{
             Binary, BinaryOp, ConstrainError, Instruction, InstructionId,
-            binary::eval_constant_binary_op,
+            binary::{convert_signed_integer_to_field_element, eval_constant_binary_op},
         },
         post_order::PostOrder,
         types::{NumericType, Type},
@@ -367,6 +367,9 @@ impl<'f> LoopInvariantContext<'f> {
     fn set_induction_var_bounds(&mut self, loop_: &Loop, current_loop: bool) {
         let bounds = loop_.get_const_bounds(self.inserter.function, self.pre_header());
         if let Some((lower_bound, upper_bound)) = bounds {
+            let bit_size = NumericType::length_type().bit_size();
+            let lower_bound = convert_signed_integer_to_field_element(lower_bound, bit_size);
+            let upper_bound = convert_signed_integer_to_field_element(upper_bound, bit_size);
             let induction_variable = loop_.get_induction_variable(self.inserter.function);
             let induction_variable = self.inserter.resolve(induction_variable);
             if current_loop {

compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs

@@ -21,7 +21,7 @@ impl Ssa {
     /// During normal compilation this is often not the case since prior passes
     /// may increase the ID counter so that later passes start at different offsets,
     /// even if they contain the same SSA code.
-    pub(crate) fn normalize_ids(&mut self) {
+    pub fn normalize_ids(&mut self) {
         let mut context = Context::default();
         context.populate_functions(&self.functions);
         for function in self.functions.values_mut() {

compiler/noirc_evaluator/src/ssa/opt/unrolling.rs

@@ -20,7 +20,7 @@
 //! only used by Brillig bytecode.
 use std::collections::BTreeSet;
 
-use acvm::{FieldElement, acir::AcirField};
+use acvm::acir::AcirField;
 use im::HashSet;
 
 use crate::{
@@ -34,8 +34,12 @@
             dom::DominatorTree,
             function::Function,
             function_inserter::{ArrayCache, FunctionInserter},
-            instruction::{Binary, BinaryOp, Instruction, InstructionId, TerminatorInstruction},
+            instruction::{
+                Binary, BinaryOp, Instruction, InstructionId, TerminatorInstruction,
+                binary::try_convert_field_element_to_signed_integer,
+            },
             post_order::PostOrder,
+            types::NumericType,
             value::ValueId,
         },
         ssa_gen::Ssa,
@@ -282,6 +286,17 @@
         Self { header, back_edge_start, blocks }
     }
 
+    /// Handle negative values represented as fields.
+    fn get_const_as_i128(function: &Function, id: ValueId) -> Option<i128> {
+        let (value, typ) = function.dfg.get_numeric_constant_with_type(id)?;
+        if matches!(typ, NumericType::NativeField) {
+            // Shouldn't happen, as fields don't have meaningful ordering.
+            value.try_into_i128()
+        } else {
+            try_convert_field_element_to_signed_integer(value, typ.bit_size())
+        }
+    }
+
     /// Find the lower bound of the loop in the pre-header and return it
     /// if it's a numeric constant, which it will be if the previous SSA
     /// steps managed to inline it.
@@ -296,13 +311,9 @@
     ///     v5 = lt v1, u32 4
     ///     jmpif v5 then: b3, else: b2
     /// ```
-    fn get_const_lower_bound(
-        &self,
-        function: &Function,
-        pre_header: BasicBlockId,
-    ) -> Option<FieldElement> {
+    fn get_const_lower_bound(&self, function: &Function, pre_header: BasicBlockId) -> Option<i128> {
         let jump_value = get_induction_variable(function, pre_header).ok()?;
-        function.dfg.get_numeric_constant(jump_value)
+        Self::get_const_as_i128(function, jump_value)
     }
 
     /// Find the upper bound of the loop in the loop header and return it
@@ -319,7 +330,7 @@
     ///     v5 = lt v1, u32 4           // Upper bound
     ///     jmpif v5 then: b3, else: b2
     /// ```
-    fn get_const_upper_bound(&self, function: &Function) -> Option<FieldElement> {
+    fn get_const_upper_bound(&self, function: &Function) -> Option<i128> {
         let block = &function.dfg[self.header];
         let instructions = block.instructions();
         if instructions.is_empty() {
@@ -336,14 +347,14 @@
 
         match &function.dfg[instructions[0]] {
             Instruction::Binary(Binary { lhs: _, operator: BinaryOp::Lt, rhs }) => {
-                function.dfg.get_numeric_constant(*rhs)
+                Self::get_const_as_i128(function, *rhs)
             }
             Instruction::Binary(Binary { lhs: _, operator: BinaryOp::Eq, rhs }) => {
                 // `for i in 0..1` is turned into:
                 // b1(v0: u32):
                 //   v12 = eq v0, u32 0
                 //   jmpif v12 then: b3, else: b2
-                function.dfg.get_numeric_constant(*rhs).map(|c| c + FieldElement::one())
+                Self::get_const_as_i128(function, *rhs).map(|c| c + 1)
             }
             other => panic!("Unexpected instruction in header: {other:?}"),
         }
@@ -354,7 +365,7 @@
         &self,
         function: &Function,
         pre_header: BasicBlockId,
-    ) -> Option<(FieldElement, FieldElement)> {
+    ) -> Option<(i128, i128)> {
         let lower = self.get_const_lower_bound(function, pre_header)?;
         let upper = self.get_const_upper_bound(function)?;
         Some((lower, upper))
@@ -678,16 +689,14 @@
     ) -> Option<BoilerplateStats> {
         let pre_header = self.get_pre_header(function, cfg).ok()?;
         let (lower, upper) = self.get_const_bounds(function, pre_header)?;
-        let lower = lower.try_to_u64()?;
-        let upper = upper.try_to_u64()?;
         let refs = self.find_pre_header_reference_values(function, cfg)?;
 
         let (loads, stores) = self.count_loads_and_stores(function, &refs);
         let increments = self.count_induction_increments(function);
         let all_instructions = self.count_all_instructions(function);
 
         Some(BoilerplateStats {
-            iterations: (upper - lower) as usize,
+            iterations: (upper - lower).max(0) as usize,
             loads,
             stores,
             increments,
@@ -1029,7 +1038,6 @@
 
 #[cfg(test)]
 mod tests {
-    use acvm::FieldElement;
     use test_case::test_case;
 
     use crate::assert_ssa_snapshot;
@@ -1041,7 +1049,7 @@
     /// Tries to unroll all loops in each SSA function once, calling the `Function` directly,
     /// bypassing the iterative loop done by the SSA which does further optimisations.
     ///
     /// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state.
     fn try_unroll_loops(mut ssa: Ssa) -> (Ssa, Vec<RuntimeError>) {
         let mut errors = vec![];
         for function in ssa.functions.values_mut() {
@@ -1160,8 +1168,8 @@
         let (lower, upper) =
             loop_.get_const_bounds(function, pre_header).expect("bounds are numeric const");
 
-        assert_eq!(lower, FieldElement::from(0u32));
-        assert_eq!(upper, FieldElement::from(4u32));
+        assert_eq!(lower, 0);
+        assert_eq!(upper, 4);
     }
 
     #[test]
@@ -1197,6 +1205,15 @@
         assert!(stats.is_small());
     }
 
+    #[test]
+    fn test_boilerplate_stats_i64() {
+        // Looping from 0..-1, but
+        let ssa = brillig_unroll_test_case_6470_with_params("i64", "0", "18446744073709551615");
+        let stats = loop0_stats(&ssa);
+        assert_eq!(stats.unrolled_instructions(), 0);
+        assert_eq!(stats.iterations, 0);
+    }
+
     #[test]
     fn test_boilerplate_stats_6470() {
         let ssa = brillig_unroll_test_case_6470(3);
@@ -1393,30 +1410,36 @@
     /// removing the `unconstrained` from the `main` function and
     /// compiling the program with `nargo --test-program . compile --show-ssa`.
     fn brillig_unroll_test_case() -> Ssa {
-        let src = "
+        brillig_unroll_test_case_with_params("u32", "0", "4")
+    }
+
+    fn brillig_unroll_test_case_with_params(idx_type: &str, lower: &str, upper: &str) -> Ssa {
+        let src = format!(
+            "
         // After `static_assert` and `assert_constant`:
-        brillig(inline) fn main f0 {
+        brillig(inline) fn main f0 {{
           b0(v0: u32):
             v2 = allocate -> &mut u32
             store u32 0 at v2
-            jmp b1(u32 0)
-          b1(v1: u32):
-            v5 = lt v1, u32 4
+            jmp b1({idx_type} {lower})
+          b1(v1: {idx_type}):
+            v5 = lt v1, {idx_type} {upper}
             jmpif v5 then: b3, else: b2
           b3():
             v8 = load v2 -> u32
             v9 = add v8, v1
             store v9 at v2
-            v11 = add v1, u32 1
+            v11 = add v1, {idx_type} 1
             jmp b1(v11)
           b2():
             v6 = load v2 -> u32
             v7 = eq v6, v0
             constrain v6 == v0
             return
-        }
-        ";
-        Ssa::from_str(src).unwrap()
+        }}
+        "
+        );
+        Ssa::from_str(&src).unwrap()
     }
 
     /// Test case from #6470:
@@ -1433,6 +1456,10 @@
     /// ```
     /// The `num_iterations` parameter can be used to make it more costly to inline.
     fn brillig_unroll_test_case_6470(num_iterations: usize) -> Ssa {
+        brillig_unroll_test_case_6470_with_params("u32", "0", &format!("{num_iterations}"))
+    }
+
+    fn brillig_unroll_test_case_6470_with_params(idx_type: &str, lower: &str, upper: &str) -> Ssa {
         let src = format!(
             "
         // After `static_assert` and `assert_constant`:
@@ -1443,18 +1470,18 @@
             inc_rc v3
             v4 = allocate -> &mut [u64; 6]
             store v3 at v4
-            jmp b1(u32 0)
-          b1(v1: u32):
-            v7 = lt v1, u32 {num_iterations}
+            jmp b1({idx_type} {lower})
+          b1(v1: {idx_type}):
+            v7 = lt v1, {idx_type} {upper}
             jmpif v7 then: b3, else: b2
           b3():
             v9 = load v4 -> [u64; 6]
             v10 = array_get v0, index v1 -> u64
             v12 = add v10, u64 1
             v13 = array_set v9, index v1, value v12
-            v15 = add v1, u32 1
+            v15 = add v1, {idx_type} 1
             store v13 at v4
-            v16 = add v1, u32 1 // duplicate
+            v16 = add v1, {idx_type} 1 // duplicate
             jmp b1(v16)
           b2():
             v8 = load v4 -> [u64; 6]

compiler/noirc_evaluator/src/ssa/parser/mod.rs

@@ -64,7 +64,7 @@ impl Ssa {
     }
 }
 
-pub(crate) struct SsaErrorWithSource {
+pub struct SsaErrorWithSource {
     src: String,
     error: SsaError,
 }

compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs

@@ -6,7 +6,7 @@
 use acvm::AcirField;
 use noirc_frontend::hir_def::expr::Constructor;
 use noirc_frontend::token::FmtStrFragment;
-pub(crate) use program::Ssa;
+pub use program::Ssa;
 
 use context::{Loop, SharedContext};
 use iter_extended::{try_vecmap, vecmap};
@@ -45,7 +45,7 @@
 /// Generates SSA for the given monomorphized program.
 ///
 /// This function will generate the SSA but does not perform any optimizations on it.
-pub(crate) fn generate_ssa(program: Program) -> Result<Ssa, RuntimeError> {
+pub fn generate_ssa(program: Program) -> Result<Ssa, RuntimeError> {
     // see which parameter has call_data/return_data attribute
     let is_databus = DataBusBuilder::is_databus(&program.main_function_signature);
 
@@ -519,7 +519,7 @@
     ///   br loop_entry(v0)
     /// loop_entry(i: Field):
     ///   v2 = lt i v1
     ///   brif v2, then: loop_body, else: loop_end
     /// loop_body():
     ///   v3 = ... codegen body ...
     ///   v4 = add 1, i
@@ -625,7 +625,7 @@
     ///   jmp while_entry()
     /// while_entry:
     ///   v0 = ... codegen cond ...
-    ///   jmpif v0, then: while_body, else: while_end  
+    ///   jmpif v0, then: while_body, else: while_end
     /// while_body():
     ///   v3 = ... codegen body ...
     ///   jmp while_entry()
@@ -663,7 +663,7 @@
     ///
     /// ```text
     ///   v0 = ... codegen cond ...
     ///   brif v0, then: then_block, else: else_block
     /// then_block():
     ///   v1 = ... codegen a ...
     ///   br end_if(v1)
@@ -678,7 +678,7 @@
     ///
     /// ```text
     ///   v0 = ... codegen cond ...
     ///   brif v0, then: then_block, else: end_if
     /// then_block:
     ///   v1 = ... codegen a ...
     ///   br end_if()

compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs

@@ -17,7 +17,7 @@ use super::ValueId;
 /// Contains the entire SSA representation of the program.
 #[serde_as]
 #[derive(Serialize, Deserialize)]
-pub(crate) struct Ssa {
+pub struct Ssa {
     #[serde_as(as = "Vec<(_, _)>")]
     pub(crate) functions: BTreeMap<FunctionId, Function>,
     pub(crate) used_globals: HashMap<FunctionId, HashSet<ValueId>>,

test_programs/execution_success/regression_8009/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "regression_8009"
+version = "0.1.0"
+type = "bin"
+authors = [""]
+
+[dependencies]

test_programs/execution_success/regression_8009/Prover.toml

@@ -0,0 +1,3 @@
+start = -51675949543456665
+end = -1
+return = 0

test_programs/execution_success/regression_8009/src/main.nr

@@ -0,0 +1,9 @@
+unconstrained fn main(start: i64, end: i64) -> pub u32 {
+    let start = (start % 5);
+    let end = (end % 5);
+    let mut sum = 0;
+    for i in start .. end {
+        sum += 1;
+    }
+    sum
+}

test_programs/execution_success/regression_8009/stdout.txt

@@ -0,0 +1 @@
+[regression_8009] Circuit output: Field(0)

tooling/ast_fuzzer/Cargo.toml

@@ -32,4 +32,5 @@ noir_fuzzer.workspace = true
 
 [dev-dependencies]
 arbtest.workspace = true
+insta.workspace = true
 rand.workspace = true