Sharing Bug Fix (#2071)

* fix

* small change

calyx-opt/src/analysis/live_range_analysis.rs

@@ -350,6 +350,7 @@ impl LiveRangeAnalysis {
         control: &mut ir::Control,
         state_share: ShareSet,
         share: ShareSet,
+        only_run_once: bool,
     ) -> Self {
         let mut ranges = LiveRangeAnalysis {
             state_share,
@@ -360,12 +361,18 @@ impl LiveRangeAnalysis {
         // Give each control statement a unique "NODE_ID" attribute.
         ControlId::compute_unique_ids(control, 0, false);
 
-        ranges.build_live_ranges(
+        let (alive, gens, kills) = ranges.build_live_ranges(
             control,
             Prop::default(),
             Prop::default(),
             Prop::default(),
         );
+        // If the component could run more than once, than we have to feed the
+        // output alive,gens,kills, back into the control and run the algorithm
+        // again.
+        if !only_run_once {
+            ranges.build_live_ranges(control, alive, gens, kills);
+        }
 
         for (node, cells_by_type) in &ranges.invokes_enables_map {
             if let Some(prop) = ranges.live.get_mut(node) {

calyx-opt/src/passes/cell_share.rs

@@ -105,6 +105,9 @@ pub struct CellShare {
     /// to the start of the par) that group is live
     par_timing_map: StaticParTiming,
 
+    //name of main (we know main will only be run once)
+    main: ir::Id,
+
     // ========= Pass Options ============
     /// The number of times a given class of cell can be shared. bounds should be
     /// length 3 to hold the 3 classes: comb cells, registers, and everything else
@@ -170,6 +173,7 @@ impl ConstructVisitor for CellShare {
             state_shareable,
             shareable,
             par_timing_map: StaticParTiming::default(),
+            main: ctx.entrypoint,
             share_freqs: HashMap::new(),
             calyx_2020: opts["calyx-2020"].bool(),
             bounds: opts["bounds"].num_list_exact::<3>(),
@@ -207,12 +211,20 @@ impl CellShare {
                 .map(|cell| cell.borrow().name()),
         );
 
+        // We know main will only ever execute once
+        // If the component is shareable, then we know it completley overwrites
+        // state at each invocation and is therefore fine to treat as if it
+        // runs once (i.e., state doesn't live beyond a single invocation).
+        let only_run_once = comp.name == self.main
+            || comp.attributes.has(ir::BoolAttr::StateShare);
+
         // TODO(rachit): Pass cont_ref_cells to LiveRangeAnalysis so that it ignores unneccessary
         // cells.
         self.live = LiveRangeAnalysis::new(
             &mut comp.control.borrow_mut(),
             self.state_shareable.clone(),
             self.shareable.clone(),
+            only_run_once,
         );
 
         self.par_timing_map = StaticParTiming::new(

tests/passes/cell-share/no-share-lifetime-beyond-invocation.expect

@@ -0,0 +1,52 @@
+import "primitives/core.futil";
+import "primitives/memories/seq.futil";
+import "primitives/binary_operators.futil";
+component memsto(@go go: 1, @clk clk: 1, @reset reset: 1) -> (@done done: 1) {
+  cells {
+    ref mem = seq_mem_d1(32, 3, 8);
+    i = std_reg(8);
+    j = std_reg(8);
+    i_incr = std_add(8);
+  }
+  wires {
+    group mem_store {
+      mem.addr0 = i.out;
+      mem.write_en = 1'd1;
+      mem.write_data = 32'd4;
+      mem.content_en = 1'd1;
+      mem_store[done] = mem.done;
+    }
+    group i_incr_group {
+      i_incr.left = i.out;
+      i_incr.right = 8'd1;
+      i.write_en = 1'd1;
+      i.in = i_incr.out;
+      i_incr_group[done] = i.done;
+    }
+    group zero_out_j {
+      j.in = 8'd0;
+      j.write_en = 1'd1;
+      zero_out_j[done] = j.done;
+    }
+  }
+  control {
+    seq {
+      mem_store;
+      i_incr_group;
+      zero_out_j;
+    }
+  }
+}
+component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (@done done: 1) {
+  cells {
+    memsto = memsto();
+    @external mem = seq_mem_d1(32, 3, 8);
+  }
+  wires {}
+  control {
+    seq {
+      invoke memsto[mem = mem]()();
+      invoke memsto[mem = mem]()();
+    }
+  }
+}

tests/passes/cell-share/no-share-lifetime-beyond-invocation.futil

@@ -0,0 +1,58 @@
+// -p cell-share -p remove-ids
+
+// Should not share i and j; even though it won't affect a single invocation,
+// it could affect the next invocation.
+import "primitives/core.futil";
+import "primitives/memories/seq.futil";
+import "primitives/binary_operators.futil";
+component memsto() -> () {
+  cells {
+    ref mem = seq_mem_d1(32, 3, 8);
+    i = std_reg(8);
+    j = std_reg(8);
+    i_incr = std_add(8);
+  }
+  wires {
+    group mem_store {
+      mem.addr0 = i.out;
+      mem.write_en = 1'd1;
+      mem.write_data = 32'd4;
+      mem.content_en = 1'd1;
+      mem_store[done] = mem.done;
+    }
+    group i_incr_group {
+      i_incr.left = i.out;
+      i_incr.right = 8'd1;
+      i.write_en = 1'd1;
+      i.in = i_incr.out;
+      i_incr_group[done] = i.done;
+    }
+    group zero_out_j {
+      j.in = 8'd0;
+      j.write_en = 1'd1;
+      zero_out_j[done] = j.done;
+    }
+  }
+  control {
+    seq {
+      mem_store;
+      i_incr_group;
+      zero_out_j;
+    }
+  }
+}
+component main() -> () {
+  cells {
+    memsto = memsto();
+    @external mem = seq_mem_d1(32, 3, 8);
+  }
+  wires {
+
+  }
+  control {
+    seq {
+      invoke memsto[mem=mem]()();
+      invoke memsto[mem=mem]()();
+    }
+  }
+}