fix: Disable if optimization

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

@@ -375,7 +375,6 @@ impl<'f> Context<'f> {
         let old_condition = *condition;
         let then_condition = self.inserter.resolve(old_condition);
 
-        let one = FieldElement::one();
         let old_stores = std::mem::take(&mut self.store_values);
         let old_allocations = std::mem::take(&mut self.local_allocations);
         let branch = ConditionalBranch {
@@ -393,15 +392,6 @@ impl<'f> Context<'f> {
         };
         self.condition_stack.push(cond_context);
         self.insert_current_side_effects_enabled();
-        // Optimization: within the then branch we know the condition to be true, so replace
-        // any references of it within this branch with true. Likewise, do the same with false
-        // with the else branch. We must be careful not to replace the condition if it is a
-        // known constant, otherwise we can end up setting 1 = 0 or vice-versa.
-        if self.inserter.function.dfg.get_numeric_constant(old_condition).is_none() {
-            let known_value = self.inserter.function.dfg.make_constant(one, Type::bool());
-
-            self.inserter.map_value(old_condition, known_value);
-        }
         vec![self.branch_ends[if_entry], *else_destination, *then_destination]
     }
 
@@ -414,7 +404,6 @@ impl<'f> Context<'f> {
             self.insert_instruction(Instruction::Not(cond_context.condition), CallStack::new());
         let else_condition = self.link_condition(else_condition);
 
-        let zero = FieldElement::zero();
         // Make sure the else branch sees the previous values of each store
         // rather than any values created in the 'then' branch.
         let old_stores = std::mem::take(&mut cond_context.then_branch.store_values);
@@ -429,21 +418,12 @@ impl<'f> Context<'f> {
             local_allocations: old_allocations,
             last_block: *block,
         };
-        let old_condition = else_branch.old_condition;
         cond_context.then_branch.local_allocations.clear();
         cond_context.else_branch = Some(else_branch);
         self.condition_stack.push(cond_context);
 
         self.insert_current_side_effects_enabled();
-        // Optimization: within the then branch we know the condition to be true, so replace
-        // any references of it within this branch with true. Likewise, do the same with false
-        // with the else branch. We must be careful not to replace the condition if it is a
-        // known constant, otherwise we can end up setting 1 = 0 or vice-versa.
-        if self.inserter.function.dfg.get_numeric_constant(old_condition).is_none() {
-            let known_value = self.inserter.function.dfg.make_constant(zero, Type::bool());
-
-            self.inserter.map_value(old_condition, known_value);
-        }
+
         assert_eq!(self.cfg.successors(*block).len(), 1);
         vec![self.cfg.successors(*block).next().unwrap()]
     }
@@ -471,11 +451,6 @@ impl<'f> Context<'f> {
         // known to be true/false within the then/else branch respectively.
         self.insert_current_side_effects_enabled();
 
-        // We must map back to `then_condition` here. Mapping `old_condition` to itself would
-        // lose any previous mappings.
-        self.inserter
-            .map_value(cond_context.then_branch.old_condition, cond_context.then_branch.condition);
-
         // While there is a condition on the stack we don't compile outside the condition
         // until it is popped. This ensures we inline the full then and else branches
         // before continuing from the end of the conditional here where they can be merged properly.

test_programs/execution_success/regression_5202/Nargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "regression_5202"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.30.0"
+
+[dependencies]
+fraction = { path = "fraction" }

test_programs/execution_success/regression_5202/fraction/LICENSE

@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2023 Resurgence Labs
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

test_programs/execution_success/regression_5202/fraction/Nargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "fraction"
+type = "lib"
+authors = [""]
+
+[dependencies]

test_programs/execution_success/regression_5202/fraction/README.md

@@ -0,0 +1,10 @@
+# fraction
+The Noir repository for accessing Fractions by Resurgence Labs
+
+# usage
+To Use as a Dependency, add the following line to your project's Nargo.toml under [dependencies]:  
+fraction = { tag = "v1.2.2", git = "https://github.com/resurgencelabs/fraction" }
+
+Note: This is a highly dynamic repository. The code will change as more features are added, both at the repository level by Resurgence Labs, as well as at the language level itself by the Noir team.
+
+

test_programs/execution_success/regression_5202/fraction/src/lib.nr

@@ -0,0 +1,201 @@
+use dep::std;
+
+global MAX: Fraction = Fraction { sign: true, num: 3050913689, den: 1 };
+global MIN: Fraction = Fraction { sign: false, num: 3050913689, den: 1 };
+global ZERO: Fraction = Fraction { sign: true, num: 0, den: 1 };
+global ONE: Fraction = Fraction { sign: true, num: 1, den: 1 };
+global NEGATIVE_ONE: Fraction = Fraction { sign: false, num: 1, den: 1 };
+
+struct Fraction {
+    sign: bool,
+    num: u32,
+    den: u32,
+}
+
+// Create a Fraction type variable without lenghtier code
+pub fn toFraction(s: bool, n: u32, d: u32) -> Fraction {
+    assert(d != 0);
+    Fraction { sign: s, num: n, den: d }
+}
+
+// Swaps the numerator and denominator
+fn invertFraction(f: Fraction) -> Fraction {
+    assert(f.num != 0);
+    Fraction { sign: f.sign, num: f.den, den: f.num }
+}
+
+// Changes the Sign of the Fraction
+fn signChangeFraction(f: Fraction) -> Fraction {
+    Fraction { sign: !f.sign, num: f.num, den: f.den }
+}
+
+// this method will only work till numerator and denominator values are under 100
+// this has been set for efficiency reasons, and will be modified once the Noir team
+// can implement dynamic limit for loops
+fn reduceFraction(f: Fraction) -> Fraction {
+    let mut a = f.num;
+    let mut b = f.den;
+    let mut j = 1;
+    let mut gcd = 1;
+
+    let min = if a > b { b } else { a };
+
+    for i in 2..100 {
+        j = i as u32;
+        if (j <= min) {
+            if (a % j == 0) & (b % j == 0) {
+                gcd = j;
+            }
+        }
+    }
+
+    Fraction { sign: f.sign, num: f.num / gcd, den: f.den / gcd }
+}
+
+// Adds two fractions
+pub fn addFraction(f1: Fraction, f2: Fraction) -> Fraction {
+    let mut an = U128::from_integer(f1.num);
+    let mut ad = U128::from_integer(f1.den);
+    let mut bn = U128::from_integer(f2.num);
+    let mut bd = U128::from_integer(f2.den);
+    let mut m = f1;
+    let mut n = f2;
+
+    if f1.sign == f2.sign {
+        if ((ad * bd > U128::from_integer(2000000000))
+            | ((an * bd + ad * bn) > U128::from_integer(2000000000))
+            | ((an * bd) > U128::from_integer(2000000000))
+            | ((ad * bn) > U128::from_integer(2000000000))) {
+            m = reduceFraction(m);
+            n = reduceFraction(n);
+        }
+        an = U128::from_integer(m.num);
+        ad = U128::from_integer(m.den);
+        bn = U128::from_integer(n.num);
+        bd = U128::from_integer(n.den);
+        if ((ad * bd > U128::from_integer(2000000000))
+            | ((an * bd + ad * bn) > U128::from_integer(2000000000))
+            | ((an * bd) > U128::from_integer(2000000000))
+            | ((ad * bn) > U128::from_integer(2000000000))) {
+            let mut ddd = (an * bd + ad * bn) / (ad * bd);
+            let mut factor = U128::from_integer(1);
+            for _ in 1..5 {
+                if ddd * U128::from_integer(10) < U128::from_integer(2000000000) {
+                    ddd *= U128::from_integer(10);
+                    factor *= U128::from_integer(10);
+                }
+            }
+            let np: u32 = U128::to_integer(((an * bd + ad * bn) * factor) / (ad * bd));
+            let fx: u32 = U128::to_integer(factor);
+            Fraction { sign: f1.sign, num: np, den: fx }
+        } else {
+            Fraction { sign: f1.sign, num: (m.num * n.den + n.num * m.den), den: m.den * n.den }
+        }
+    } else if ((an * bd) > (bn * ad)) {
+        if ((ad * bd > U128::from_integer(2000000000))
+            | ((an * bd - ad * bn) > U128::from_integer(2000000000))
+            | ((an * bd) > U128::from_integer(2000000000))) {
+            m = reduceFraction(m);
+            n = reduceFraction(n);
+        }
+        an = U128::from_integer(m.num);
+        ad = U128::from_integer(m.den);
+        bn = U128::from_integer(n.num);
+        bd = U128::from_integer(n.den);
+
+        if ((ad * bd > U128::from_integer(2000000000))
+            | ((an * bd - ad * bn) > U128::from_integer(2000000000))
+            | ((an * bd) > U128::from_integer(2000000000))) {
+            let mut ddd = (an * bd - ad * bn) / (ad * bd);
+            let mut factor = U128::from_integer(1);
+            for _ in 1..5 {
+                if ddd * U128::from_integer(10) < U128::from_integer(2000000000) {
+                    ddd *= U128::from_integer(10);
+                    factor *= U128::from_integer(10);
+                }
+            }
+            let np: u32 = U128::to_integer(((an * bd - ad * bn) * factor) / (ad * bd));
+            let fx: u32 = U128::to_integer(factor);
+            Fraction { sign: f1.sign, num: np, den: fx }
+        } else {
+            Fraction { sign: f1.sign, num: (m.num * n.den - n.num * m.den), den: m.den * n.den }
+        }
+    } else {
+        if ((ad * bd > U128::from_integer(2000000000))
+            | ((bn * ad - bd * an) > U128::from_integer(2000000000))
+            | ((bn * ad) > U128::from_integer(2000000000))) {
+            m = reduceFraction(m);
+            n = reduceFraction(n);
+        }
+        an = U128::from_integer(m.num);
+        ad = U128::from_integer(m.den);
+        bn = U128::from_integer(n.num);
+        bd = U128::from_integer(n.den);
+        if ((ad * bd > U128::from_integer(2000000000))
+            | ((bn * ad - bd * an) > U128::from_integer(2000000000))
+            | ((bn * ad) > U128::from_integer(2000000000))) {
+            let mut ddd = (bn * ad - bd * an) / (ad * bd);
+            let mut factor = U128::from_integer(1);
+            for _ in 1..5 {
+                if ddd * U128::from_integer(10) < U128::from_integer(2000000000) {
+                    ddd *= U128::from_integer(10);
+                    factor *= U128::from_integer(10);
+                }
+            }
+            let np: u32 = U128::to_integer(((bn * ad - bd * an) * factor) / (ad * bd));
+            let fx: u32 = U128::to_integer(factor);
+            Fraction { sign: f2.sign, num: np, den: fx }
+        } else {
+            Fraction { sign: f2.sign, num: (n.num * m.den - m.num * n.den), den: m.den * n.den }
+        }
+    }
+}
+
+// Returns the closest but smaller Integer to the Given Fraction, but typecast to Fraction for convenience
+pub fn floor(f: Fraction) -> Fraction {
+    let q = f.num / f.den;
+    if q * f.den == f.num {
+        Fraction { sign: f.sign, num: f.num, den: f.den }
+    } else if f.sign {
+        Fraction { sign: f.sign, num: q, den: 1 }
+    } else {
+        Fraction { sign: f.sign, num: q + 1, den: 1 }
+    }
+}
+
+#[test]
+fn test_sum() {
+    let f1 = toFraction(true, 3, 5);
+    let f2 = toFraction(true, 2, 5);
+    let f = addFraction(f1, f2);
+    assert(f.num == f.den);
+}
+
+#[test]
+fn test_reduce() {
+    let f1 = toFraction(true, 2, 10);
+    let f2 = reduceFraction(f1);
+    assert(f2.num == 1);
+}
+
+#[test]
+fn test_floor() {
+    let f = toFraction(true, 7, 5);
+    let fl = floor(f);
+    assert(fl.num == 1);
+    assert(fl.den == 1);
+}
+
+#[test]
+fn test_floor2() {
+    let f = toFraction(false, 12, 5);
+    let fl = floor(f);
+    assert(fl.num == 3);
+    assert(fl.den == 1);
+}
+
+#[test]
+fn test_globals() {
+    let a = addFraction(ONE, NEGATIVE_ONE);
+    assert(a.num == ZERO.num);
+}

test_programs/execution_success/regression_5202/src/main.nr

@@ -0,0 +1,23 @@
+use dep::fraction::{Fraction, MAX, floor, toFraction, addFraction};
+
+fn main() {
+    let g1 = toFraction(true, 33333333, 5);
+    let g2 = toFraction(true, 500000, 33333333);
+    let a = addFraction(g1, g2);
+
+    let f1 = floor(a);
+    let f2 = MAX;
+    assert(f1.sign);
+    assert(f2.sign);
+
+    if f1.sign != f2.sign {
+        if (f1.sign) { () } else { () }
+    } else {
+        // Test fails here before the fix to #5202.
+        // An optimization which assumes an if condition to be true/false
+        // within the then/else branch respectively wasn't being set properly
+        // causing f1.sign to be assumed to be false in this else branch.
+        assert(f1.sign);
+        assert(f2.sign);
+    }
+}