Unstable if/whiles are errored (#1467)

* unstable if/whiles are errored

* check for static ifs too

* formatting

* small changes

* rewrote test cases

* edit tcam

* fixed tcam

* fixed mlir test

calyx-ir/src/structure.rs

@@ -97,6 +97,32 @@ impl Port {
     pub fn canonical(&self) -> Canonical {
         Canonical(self.get_parent_name(), self.name)
     }
+
+    /// Returns the value of an attribute if present
+    pub fn get_attribute<A>(&self, attr: A) -> Option<u64>
+    where
+        A: Into<Attribute>,
+    {
+        self.get_attributes().get(attr)
+    }
+
+    /// Returns true if the node has a specific attribute
+    pub fn has_attribute<A>(&self, attr: A) -> bool
+    where
+        A: Into<Attribute>,
+    {
+        self.get_attributes().has(attr)
+    }
+}
+
+impl GetAttributes for Port {
+    fn get_attributes(&self) -> &Attributes {
+        &self.attributes
+    }
+
+    fn get_mut_attributes(&mut self) -> &mut Attributes {
+        &mut self.attributes
+    }
 }
 
 impl PartialEq for Port {

calyx-opt/src/passes/well_formed.rs

@@ -529,6 +529,29 @@ impl Visitor for WellFormed {
             })?;
             // Push the combinational group to the stack of active groups
             self.active_comb.push(assigns);
+        } else if !s.port.borrow().has_attribute(ir::BoolAttr::Stable) {
+            let msg = s.attributes.copy_span().format(format!(
+                    "If statement has no comb group and its condition port {} is unstable",
+                    s.port.borrow().canonical()
+                ));
+            return Err(calyx_utils::Error::malformed_control(msg));
+        }
+        Ok(Action::Continue)
+    }
+
+    fn start_static_if(
+        &mut self,
+        s: &mut ir::StaticIf,
+        _comp: &mut Component,
+        _sigs: &LibrarySignatures,
+        _comps: &[ir::Component],
+    ) -> VisResult {
+        if !s.port.borrow().has_attribute(ir::BoolAttr::Stable) {
+            let msg = s.attributes.copy_span().format(format!(
+                "Static If statement's condition port {} is unstable",
+                s.port.borrow().canonical()
+            ));
+            Err(calyx_utils::Error::malformed_control(msg))?
         }
         Ok(Action::Continue)
     }
@@ -573,6 +596,12 @@ impl Visitor for WellFormed {
             })?;
             // Push the combinational group to the stack of active groups
             self.active_comb.push(assigns);
+        } else if !s.port.borrow().has_attribute(ir::BoolAttr::Stable) {
+            let msg = s.attributes.copy_span().format(format!(
+                    "While loop has no comb group and its condition port {} is unstable",
+                    s.port.borrow().canonical()
+                ));
+            return Err(calyx_utils::Error::malformed_control(msg));
         }
         Ok(Action::Continue)
     }

primitives/tcam.futil

@@ -140,6 +140,9 @@ component comparator_element(lenA: 5, lenB: 5, addrA: 5, addrB: 5, mlA: 1, mlB:
 // invalid searches will be defaulted to index zero.
 component TCAM_IPv4(write_en: 1, search_en: 1, in: 32, prefix_len: 6, write_index: 5) -> (index: 5) {
   cells {
+    write_en_reg = std_reg(1);
+    search_en_reg = std_reg(1);
+    ce40_reg = std_reg(1);
     p0 = std_reg(32);
     p1 = std_reg(32);
     p2 = std_reg(32);
@@ -349,6 +352,21 @@ component TCAM_IPv4(write_en: 1, search_en: 1, in: 32, prefix_len: 6, write_inde
     out = std_reg(5);
   }
   wires {
+    group write_write_en_reg {
+      write_en_reg.write_en = 1'd1; 
+      write_en_reg.in = write_en; 
+      write_write_en_reg[done] = write_en_reg.done; 
+    }
+    group write_search_en_reg {
+      search_en_reg.write_en = 1'd1; 
+      search_en_reg.in = search_en; 
+      write_search_en_reg[done] = search_en_reg.done; 
+    }
+    group write_ce40_reg {
+      ce40_reg.write_en = 1'd1; 
+      ce40_reg.in = ce40.mlX;
+      write_ce40_reg[done] = ce40_reg.done; 
+    }
     comb group is_length_zero {
       z_eq.left = 6'd0;
       z_eq.right = prefix_len;
@@ -824,8 +842,10 @@ component TCAM_IPv4(write_en: 1, search_en: 1, in: 32, prefix_len: 6, write_inde
   }
 
   control {
+    write_write_en_reg; 
+    write_search_en_reg;
     par {
-      if write_en {
+      if write_en_reg.out {
         if z_eq.out with is_length_zero {
           write_zero;
         } else {
@@ -868,7 +888,7 @@ component TCAM_IPv4(write_en: 1, search_en: 1, in: 32, prefix_len: 6, write_inde
           }
         }
       }
-      if search_en {
+      if search_en_reg.out {
         seq {
           par {
             invoke me0(in=in, prefix=p0.out, length=l0.out)();
@@ -943,10 +963,11 @@ component TCAM_IPv4(write_en: 1, search_en: 1, in: 32, prefix_len: 6, write_inde
             invoke ce31(lenA=ce22.lenX, lenB=ce23.lenX, addrA=ce22.addrX, addrB=ce23.addrX, mlA=ce22.mlX, mlB=ce23.mlX)();
           }
           invoke ce40(lenA=ce30.lenX, lenB=ce31.lenX, addrA=ce30.addrX, addrB=ce31.addrX, mlA=ce30.mlX, mlB=ce31.mlX)();
+          write_ce40_reg;
           // If the final comparator has a valid value then save it.
-          if ce40.mlX { save_index; } else { default_to_zero_length_index; }
+          if ce40_reg.out { save_index; } else { default_to_zero_length_index; }
         }
       }
     }
   }
-}
+}

tests/backend/mlir/with-guards.expect

@@ -19,6 +19,7 @@ calyx.component @B(%in: i1, %go: i1 {go=1}, %clk: i1 {clk=1}, %reset: i1 {reset=
 calyx.component @main(%go: i1 {go=1}, %clk: i1 {clk=1}, %reset: i1 {reset=1}) -> (%out: i1, %done: i1 {done=1}) {
   %c0.in, %c0.go, %c0.clk, %c0.reset, %c0.out, %c0.flag, %c0.done = calyx.instance @c0 of @A : i8, i1, i1, i1, i8, i1, i1
   %c1.in, %c1.go, %c1.clk, %c1.reset, %c1.out, %c1.done = calyx.instance @c1 of @B : i1, i1, i1, i1, i1, i1
+  %c1_res.in, %c1_res.write_en, %c1_res.clk, %c1_res.reset, %c1_res.out, %c1_res.done = calyx.register @c1_res : i1, i1, i1, i1, i1, i1
   %r1.in, %r1.write_en, %r1.clk, %r1.reset, %r1.out, %r1.done = calyx.register @r1 : i8, i1, i1, i1, i8, i1
   %r2.in, %r2.write_en, %r2.clk, %r2.reset, %r2.out, %r2.done = calyx.register @r2 : i1, i1, i1, i1, i1, i1
   %m0.addr0, %m0.write_data, %m0.write_en, %m0.clk, %m0.reset, %m0.read_data, %m0.done = calyx.memory @m0 <[1] x 32> [1] : i1, i32, i1, i1, i1, i32, i1
@@ -27,18 +28,23 @@ calyx.component @main(%go: i1 {go=1}, %clk: i1 {clk=1}, %reset: i1 {reset=1}) ->
   %a0.left, %a0.right, %a0.out = calyx.std_add @a0 : i32, i32, i32
   %s0.in, %s0.out = calyx.std_slice @s0 : i32, i8
   %add.left, %add.right, %add.out = calyx.std_add @add : i8, i8, i8
+  %_1_1.out = hw.constant 1 : i1
   %_0_1.out = hw.constant 0 : i1
   %_1_32.out = hw.constant 1 : i32
   %_1_3.out = hw.constant 1 : i3
   %_2_3.out = hw.constant 2 : i3
   %_3_3.out = hw.constant 3 : i3
   %_1_8.out = hw.constant 1 : i8
-  %_1_1.out = hw.constant 1 : i1
   %or0.left, %or0.right, %or0.out = calyx.std_or @or0 {generated=1} : i1, i1, i1
   %and0.left, %and0.right, %and0.out = calyx.std_and @and0 {generated=1} : i1, i1, i1
   %or1.left, %or1.right, %or1.out = calyx.std_or @or1 {generated=1} : i1, i1, i1
   %and1.left, %and1.right, %and1.out = calyx.std_and @and1 {generated=1} : i1, i1, i1
   calyx.wires {
+    calyx.group @write_c1_res {
+      calyx.assign %c1_res.write_en = %_1_1.out ? %_1_1.out : i1
+      calyx.assign %c1_res.in = %_1_1.out ? %c1.out : i1
+      calyx.group_done %_1_1.out ? %c1_res.done : i1
+    }
     calyx.group @Group1 {
       calyx.assign %s0.in = %_1_1.out ? %a0.out : i32
       calyx.assign %m0.addr0 = %_1_1.out ? %_0_1.out : i1
@@ -76,10 +82,11 @@ calyx.component @main(%go: i1 {go=1}, %clk: i1 {clk=1}, %reset: i1 {reset=1}) ->
         calyx.seq {
           calyx.enable @Group1
           calyx.enable @Group1
-          calyx.if %c1.out {
+          calyx.enable @write_c1_res
+          calyx.if %c1_res.out {
             calyx.enable @Group2
           }
-          calyx.if %c1.out {
+          calyx.if %c1_res.out {
             calyx.enable @Group2
           }
         }

tests/backend/mlir/with-guards.futil

@@ -20,6 +20,7 @@ component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (out: 1, @done done:
   cells {
     c0 = A();
     c1 = B();
+    c1_res = std_reg(1);
     r1 = std_reg(8);
     r2 = std_reg(1);
 
@@ -31,6 +32,11 @@ component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (out: 1, @done done:
     add = std_add(8);
   }
   wires {
+    group write_c1_res {
+      c1_res.write_en = 1'd1; 
+      c1_res.in = c1.out; 
+      write_c1_res[done] = c1_res.done; 
+    }
     group Group1 {
       s0.in = a0.out;
       m0.addr0 = 1'd0;
@@ -59,10 +65,11 @@ component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (out: 1, @done done:
         seq {
           Group1;
           Group1;
-          if c1.out {
+          write_c1_res;
+          if c1_res.out {
             Group2;
           }
-          if c1.out {
+          if c1_res.out {
             Group2;
           }
         }

tests/correctness/pipelined-mac.futil

@@ -21,6 +21,7 @@ component pipelined_mac(
 
     stage2_valid = std_reg(1); // Stage 2 should run this execution
     out_valid = std_reg(1);    // Output is valid
+    data_valid_reg = std_reg(1); // Stores value of data_valid 
   }
   wires {
     group stage1<"static"=4> {
@@ -59,20 +60,26 @@ component pipelined_mac(
       out_valid.write_en = 1'd1;
       unset_out_valid[done] = out_valid.done;
     }
+    group write_data_valid {
+      data_valid_reg.write_en = 1'd1; 
+      data_valid_reg.in = data_valid; 
+      write_data_valid[done] = data_valid_reg.done;
+    }
 
     output_valid = out_valid.out;
     out = pipe2.out;
   }
   control {
     seq {
+      write_data_valid;
       // Execute all stages in parallel
       par {
-        if data_valid { stage1; }
+        if data_valid_reg.out { stage1; }
         if stage2_valid.out { stage2; }
       }
       // Configure valid signals for next invoke
       par {
-        if data_valid {
+        if data_valid_reg.out {
           set_stage2_valid;
         } else {
           unset_stage2_valid;

tests/correctness/static-control/if-start.expect

@@ -1,5 +1,5 @@
 {
-  "cycles": 4,
+  "cycles": 5,
   "memories": {
     "cond": [
       0

tests/correctness/static-control/if-start.futil

@@ -6,9 +6,14 @@ component main() -> () {
       @external cond = std_mem_d1(1, 1, 1);
       @external m = std_mem_d1(32, 1, 1);
       add = std_add(32);
-      r = std_reg(1);
+      read_cond_reg = std_reg(1);
     }
     wires {
+        static<1> group read_cond {
+            cond.addr0 = 1'd0;
+            read_cond_reg.write_en = 1'd1; 
+            read_cond_reg.in = cond.read_data;
+        }
         static<1> group one {
             add.left = m.read_data;
             add.right = 32'd1;
@@ -18,13 +23,15 @@ component main() -> () {
         }
         static<4> group four {
         }
-        cond.addr0 = 1'd0;
     }
     control {
-        static if cond.read_data {
-            four;
-        } else {
-            one;
+        static seq {
+            read_cond; 
+            static if read_cond_reg.out {
+                four;
+            } else {
+                one;
+            }
         }
     }
 }

tests/correctness/static/if-start.expect

@@ -1,5 +1,5 @@
 {
-  "cycles": 4,
+  "cycles": 5,
   "memories": {
     "cond": [
       0

tests/correctness/static/if-start.futil

@@ -6,6 +6,7 @@ component main() -> () {
       @external m = std_mem_d1(32, 1, 1);
       add = std_add(32);
       r = std_reg(1);
+      read_cond_reg = std_reg(1);
     }
     wires {
         group one {
@@ -19,10 +20,17 @@ component main() -> () {
         group four<"static"=4> {
             four[done] = r.out;
         }
-        cond.addr0 = 1'd0;
+        group read_cond {
+            cond.addr0 = 1'd0;
+            read_cond_reg.write_en = 1'd1; 
+            read_cond_reg.in = cond.read_data;
+            read_cond[done] = read_cond_reg.done; 
+        }
+
     }
     control {
-        if cond.read_data {
+        read_cond; 
+        if read_cond_reg.out {
             four;
         } else {
             one;

tests/correctness/static/static-mult-dot-product.futil

@@ -11,6 +11,7 @@ component main() -> () {
         add = std_add(4);
         sub = std_sub(4);
         ud = undef(1);
+        meaningless_reg = std_reg(1);
 
         lt = std_lt(4);
         lt_10 = std_reg(1);
@@ -80,7 +81,7 @@ component main() -> () {
         par { start_mult; incr; }
         par { start_mult; incr; }
         par { start_mult; incr; }
-        @bound(6) while ud.out {
+        @bound(6) while meaningless_reg.out {
             par {
                 incr;
                 start_mult;

tests/errors/comb-port-in-condition.expect

@@ -0,0 +1,6 @@
+---CODE---
+1
+---STDERR---
+Error: Malformed Control: tests/errors/comb-port-in-condition.futil
+8 |    if le.out { seq {} }
+  |    ^^^^^^^^^^^^^^^^^^^^ If statement has no comb group and its condition port le.out is unstable