diff --git a/calyx-frontend/src/attribute.rs b/calyx-frontend/src/attribute.rs index e50ece67d5..7db2a9078f 100644 --- a/calyx-frontend/src/attribute.rs +++ b/calyx-frontend/src/attribute.rs @@ -69,7 +69,11 @@ pub enum BoolAttr { #[strum(serialize = "promoted")] /// denotes a static component or control promoted from dynamic Promoted, + #[strum(serialize = "fast")] + /// https://github.com/calyxir/calyx/issues/1828 + Fast, } + impl From for Attribute { fn from(attr: BoolAttr) -> Self { Attribute::Bool(attr) diff --git a/calyx-opt/src/passes/static_promotion.rs b/calyx-opt/src/passes/static_promotion.rs index b13c116955..00a64c323c 100644 --- a/calyx-opt/src/passes/static_promotion.rs +++ b/calyx-opt/src/passes/static_promotion.rs @@ -5,7 +5,7 @@ use crate::traversal::{ Action, ConstructVisitor, Named, Order, ParseVal, PassOpt, VisResult, Visitor, }; -use calyx_ir::{self as ir, LibrarySignatures}; +use calyx_ir::{self as ir, BoolAttr, LibrarySignatures}; use calyx_utils::CalyxResult; use ir::GetAttributes; use itertools::Itertools; @@ -490,6 +490,12 @@ impl Visitor for StaticPromotion { }) }; self.inference_analysis.fixup_ctrl(&mut new_ctrl); + + // this might be part of a larger issue where passes remove some attributes they shouldn't + if s.get_attributes().has(BoolAttr::Fast) { + new_ctrl.get_mut_attributes().insert(BoolAttr::Fast, 1); + } + Ok(Action::change(new_ctrl)) } diff --git a/calyx-opt/src/passes/top_down_compile_control.rs b/calyx-opt/src/passes/top_down_compile_control.rs index 398f473332..927bb188dd 100644 --- a/calyx-opt/src/passes/top_down_compile_control.rs +++ b/calyx-opt/src/passes/top_down_compile_control.rs @@ -711,6 +711,8 @@ impl Schedule<'_, '_> { preds: Vec, // True if early_transitions are allowed early_transitions: bool, + // True if the `@fast` attribute has successfully been applied to the parent of this control + has_fast_guarantee: bool, ) -> CalyxResult> { match con { // See explanation of FSM states generated in [ir::TopDownCompileControl]. @@ -746,7 +748,7 @@ impl Schedule<'_, '_> { // NOTE: We explicilty do not add `not_done` to the guard. // See explanation in [ir::TopDownCompileControl] to understand // why. - if early_transitions { + if early_transitions || has_fast_guarantee { for (st, g) in &prev_states { let early_go = build_assignments!(self.builder; group["go"] = g ? signal_on["out"]; @@ -793,9 +795,13 @@ impl Schedule<'_, '_> { early_transitions: bool, ) -> CalyxResult> { let mut prev = preds; - for stmt in &seq.stmts { - prev = - self.calculate_states_recur(stmt, prev, early_transitions)?; + for (i, stmt) in seq.stmts.iter().enumerate() { + prev = self.calculate_states_recur( + stmt, + prev, + early_transitions, + i > 0 && seq.get_attributes().has(BoolAttr::Fast), + )?; } Ok(prev) } @@ -828,6 +834,7 @@ impl Schedule<'_, '_> { &if_stmt.tbranch, tru_transitions, early_transitions, + false, )?; // Previous states transitioning into false branch need the conditional // to be false. @@ -845,6 +852,7 @@ impl Schedule<'_, '_> { &if_stmt.fbranch, fal_transitions, early_transitions, + false, )? }; @@ -887,6 +895,7 @@ impl Schedule<'_, '_> { &while_stmt.body, transitions, early_transitions, + false, )?; // Step 3: The final out edges from the while come from: @@ -991,6 +1000,7 @@ impl Schedule<'_, '_> { con, vec![first_state], early_transitions, + false, )?; self.add_nxt_transition(prev); Ok(()) diff --git a/calyx-opt/src/passes/well_formed.rs b/calyx-opt/src/passes/well_formed.rs index f3f14bcf77..32d14bb63d 100644 --- a/calyx-opt/src/passes/well_formed.rs +++ b/calyx-opt/src/passes/well_formed.rs @@ -4,6 +4,7 @@ use calyx_ir::{ self as ir, Cell, CellType, Component, GetAttributes, LibrarySignatures, RESERVED_NAMES, }; +use calyx_ir::{BoolAttr, Seq}; use calyx_utils::{CalyxResult, Error, WithPos}; use ir::Nothing; use ir::StaticTiming; @@ -282,6 +283,30 @@ fn subtype(cell_out: &Cell, cell_in: &Cell) -> bool { true } +/// Confirms (in agreement with [this discussion](https://github.com/calyxir/calyx/issues/1828)) +/// that the `@fast` sequence `seq` is composed of alternating static-dynamic controls. +fn check_fast_seq_invariant(seq: &Seq) -> CalyxResult<()> { + if seq.stmts.is_empty() { + return Ok(()); + } + let mut last_is_static = seq + .stmts + .first() + .expect("non-empty already asserted") + .is_static(); + for stmt in seq.stmts.iter().skip(1) { + if stmt.is_static() == last_is_static { + return Err( + Error::malformed_control( + "`seq` marked `@fast` does not contain alternating static-dynamic control children (see #1828)" + ) + ); + } + last_is_static = stmt.is_static(); + } + Ok(()) +} + impl Visitor for WellFormed { fn start( &mut self, @@ -741,6 +766,20 @@ impl Visitor for WellFormed { Ok(Action::Continue) } + fn start_seq( + &mut self, + s: &mut calyx_ir::Seq, + _comp: &mut Component, + _sigs: &LibrarySignatures, + _comps: &[calyx_ir::Component], + ) -> VisResult { + if s.attributes.has(BoolAttr::Fast) { + check_fast_seq_invariant(s)?; + } + + Ok(Action::Continue) + } + fn finish_if( &mut self, s: &mut ir::If, diff --git a/runt.toml b/runt.toml index 61e3cb1906..234ac1d9b1 100644 --- a/runt.toml +++ b/runt.toml @@ -287,9 +287,7 @@ timeout = 120 [[tests]] name = "correctness static timing one-hot encoding" -paths = [ - "tests/correctness/static-interface/*.futil", -] +paths = ["tests/correctness/static-interface/*.futil"] cmd = """ fud exec --from calyx --to jq \ --through verilog \ @@ -444,6 +442,32 @@ fud e --from systolic --to jq \ """ expect_dir = "tests/correctness/systolic/mmult-expect" +[[tests]] +name = "[correctness] `@fast` attribute should not change program behavior" +paths = ["tests/correctness/fast-attr/valid/*.futil"] +cmd = """ +fud exec --from calyx --to jq \ + --through dat \ + --through icarus-verilog \ + -s calyx.flags "-p validate -p compile -p lower -d static-promotion" \ + -s verilog.data {}.data \ + -s jq.expr "." \ + {} -q +""" + +[[tests]] +name = "[correctness] `@fast` attribute should reject unoptimizable sequences" +paths = ["tests/correctness/fast-attr/invalid/*.futil"] +cmd = """ +fud exec --from calyx --to jq \ + --through dat \ + --through icarus-verilog \ + -s calyx.flags "-p validate -p compile -p lower -d static-promotion" \ + -s verilog.data {}.data \ + -s jq.expr "." \ + {} -q +""" + [[tests]] name = "[frontend] systolic array relu static correctness" paths = ["tests/correctness/systolic/relu-inputs/*.data"] diff --git a/tests/correctness/fast-attr/invalid/invalid.expect b/tests/correctness/fast-attr/invalid/invalid.expect new file mode 100644 index 0000000000..d39716899f --- /dev/null +++ b/tests/correctness/fast-attr/invalid/invalid.expect @@ -0,0 +1,9 @@ +---CODE--- +255 +---STDERR--- +[fud] ERROR: `/home/calyx/target/debug/calyx -l /home/calyx -b verilog --disable-verify -p validate -p compile -p lower -d static-promotion' failed: +=====STDERR===== +Error: Malformed Control: `seq` marked `@fast` does not contain alternating static-dynamic control children (see #1828) + +=====STDOUT===== + diff --git a/tests/correctness/fast-attr/invalid/invalid.futil b/tests/correctness/fast-attr/invalid/invalid.futil new file mode 100644 index 0000000000..bedbd75fc8 --- /dev/null +++ b/tests/correctness/fast-attr/invalid/invalid.futil @@ -0,0 +1,46 @@ +import "primitives/core.futil"; +import "primitives/memories/comb.futil"; + +component main(@go go: 1) -> (@done done: 1) { + cells { + @external m = comb_mem_d1(32, 1, 32); + i0 = std_reg(32); + add = std_add(32); + } + wires { + static<1> group init { + i0.in = 32'd0; + i0.write_en = 1'b1; + } + group dyn_inc { + add.left = i0.out; + add.right = 32'd1; + i0.in = add.out; + i0.write_en = 1'b1; + dyn_inc[done] = i0.done; + } + static<1> group static_inc { + add.left = i0.out; + add.right = 32'd1; + i0.in = add.out; + i0.write_en = 1'b1; + } + group write { + m.write_data = i0.out; + m.write_en = 1'b1; + write[done] = m.done; + } + } + control { + @fast seq { + init; + dyn_inc; + dyn_inc; + dyn_inc; + static_inc; + static_inc; + static_inc; + write; + } + } +} diff --git a/tests/correctness/fast-attr/invalid/invalid.futil.data b/tests/correctness/fast-attr/invalid/invalid.futil.data new file mode 100644 index 0000000000..478b067a84 --- /dev/null +++ b/tests/correctness/fast-attr/invalid/invalid.futil.data @@ -0,0 +1,12 @@ +{ + "m": { + "data": [ + 0 + ], + "format": { + "numeric_type": "bitnum", + "is_signed": false, + "width": 32 + } + } +} diff --git a/tests/correctness/fast-attr/valid/fast6inc.expect b/tests/correctness/fast-attr/valid/fast6inc.expect new file mode 100644 index 0000000000..b9f64f9677 --- /dev/null +++ b/tests/correctness/fast-attr/valid/fast6inc.expect @@ -0,0 +1,8 @@ +{ + "cycles": 9, + "memories": { + "m": [ + 6 + ] + } +} diff --git a/tests/correctness/fast-attr/valid/fast6inc.futil b/tests/correctness/fast-attr/valid/fast6inc.futil new file mode 100644 index 0000000000..4579d81e5c --- /dev/null +++ b/tests/correctness/fast-attr/valid/fast6inc.futil @@ -0,0 +1,46 @@ +import "primitives/core.futil"; +import "primitives/memories/comb.futil"; + +component main(@go go: 1) -> (@done done: 1) { + cells { + @external m = comb_mem_d1(32, 1, 32); + i0 = std_reg(32); + add = std_add(32); + } + wires { + static<1> group init { + i0.in = 32'd0; + i0.write_en = 1'b1; + } + group dyn_inc { + add.left = i0.out; + add.right = 32'd1; + i0.in = add.out; + i0.write_en = 1'b1; + dyn_inc[done] = i0.done; + } + static<1> group static_inc { + add.left = i0.out; + add.right = 32'd1; + i0.in = add.out; + i0.write_en = 1'b1; + } + group write { + m.write_data = i0.out; + m.write_en = 1'b1; + write[done] = m.done; + } + } + control { + @fast seq { + init; + dyn_inc; + static_inc; + dyn_inc; + static_inc; + dyn_inc; + static_inc; + write; + } + } +} diff --git a/tests/correctness/fast-attr/valid/fast6inc.futil.data b/tests/correctness/fast-attr/valid/fast6inc.futil.data new file mode 100644 index 0000000000..478b067a84 --- /dev/null +++ b/tests/correctness/fast-attr/valid/fast6inc.futil.data @@ -0,0 +1,12 @@ +{ + "m": { + "data": [ + 0 + ], + "format": { + "numeric_type": "bitnum", + "is_signed": false, + "width": 32 + } + } +}