fix(ssa): delete instructions with false predicate (#760)

* Delete instructions with false predicate * add regression test --------- Co-authored-by: kevaundray <[email protected]>
noir-lang · Feb 8, 2023 · f329379 · f329379
1 parent c072150
commit f329379
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 8 deletions.
diff --git a/crates/nargo/tests/test_data/9_conditional/src/main.nr b/crates/nargo/tests/test_data/9_conditional/src/main.nr
@@ -197,6 +197,9 @@ fn main(a: u32, mut c: [u32; 4], x: [u8; 5], result: pub [u8; 32]){
         c_661 = issue_661_foo(issue_661_bar(c), x);
     }
     constrain c_661[0] < 20000;
+
+    // Regression for predicate simplification
+    safe_inverse(0);
 }
 
 fn test5(a : u32) {
@@ -249,4 +252,15 @@ fn issue_661_bar(a : [u32;4]) ->[u32;4] {
     let mut b:[u32;4] = [0;4];
     b[0]=a[0]+1;
     b
-}
+}
+
+
+fn safe_inverse(n: Field) -> Field
+{
+    if n == 0 {
+        0
+    }
+    else {
+        1 / n
+    }
+}
diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs
@@ -703,6 +703,10 @@ impl Binary {
         Binary::new(operator, lhs, rhs)
     }
 
+    fn zero_div_error(&self) -> Result<(), RuntimeError> {
+        Err(RuntimeErrorKind::Spanless("Panic - division by zero".to_string()).into())
+    }
+
     fn evaluate<F>(
         &self,
         ctx: &SsaContext,
@@ -723,8 +727,6 @@ impl Binary {
 
         let l_is_zero = lhs.map_or(false, |x| x.is_zero());
         let r_is_zero = rhs.map_or(false, |x| x.is_zero());
-        let zero_div_error =
-            Err(RuntimeErrorKind::Spanless("Panic - division by zero".to_string()).into());
 
         match &self.operator {
             BinaryOp::Add | BinaryOp::SafeAdd => {
@@ -770,7 +772,7 @@ impl Binary {
 
             BinaryOp::Udiv => {
                 if r_is_zero {
-                    return zero_div_error;
+                    self.zero_div_error()?;
                 } else if l_is_zero {
                     return Ok(l_eval); //TODO should we ensure rhs != 0 ???
                 }
@@ -783,7 +785,7 @@ impl Binary {
             }
             BinaryOp::Div => {
                 if r_is_zero {
-                    return zero_div_error;
+                    self.zero_div_error()?;
                 } else if l_is_zero {
                     return Ok(l_eval); //TODO should we ensure rhs != 0 ???
                 } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) {
@@ -792,7 +794,7 @@ impl Binary {
             }
             BinaryOp::Sdiv => {
                 if r_is_zero {
-                    return zero_div_error;
+                    self.zero_div_error()?;
                 } else if l_is_zero {
                     return Ok(l_eval); //TODO should we ensure rhs != 0 ???
                 } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) {
@@ -803,7 +805,7 @@ impl Binary {
             }
             BinaryOp::Urem => {
                 if r_is_zero {
-                    return zero_div_error;
+                    self.zero_div_error()?;
                 } else if l_is_zero {
                     return Ok(l_eval); //TODO what is the correct result?
                 } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) {
@@ -812,7 +814,7 @@ impl Binary {
             }
             BinaryOp::Srem => {
                 if r_is_zero {
-                    return zero_div_error;
+                    self.zero_div_error()?;
                 } else if l_is_zero {
                     return Ok(l_eval); //TODO what is the correct result?
                 } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) {

diff --git a/crates/noirc_evaluator/src/ssa/optimizations.rs b/crates/noirc_evaluator/src/ssa/optimizations.rs
@@ -21,6 +21,12 @@ pub fn simplify(ctx: &mut SsaContext, ins: &mut Instruction) -> Result<(), Runti
     if ins.is_deleted() {
         return Ok(());
     }
+    if let Operation::Binary(bin) = &ins.operation {
+        if bin.predicate == Some(ctx.zero_with_type(ObjectType::Boolean)) {
+            ins.mark = Mark::Deleted;
+            return Ok(());
+        }
+    }
     //1. constant folding
     let new_id = ins.evaluate(ctx)?.to_index(ctx);