Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@fast attribute to enable early (0-cycle) transitions between alternating dynamic/static groups #2118

Merged
merged 11 commits into from
Aug 7, 2024
4 changes: 4 additions & 0 deletions calyx-frontend/src/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<BoolAttr> for Attribute {
fn from(attr: BoolAttr) -> Self {
Attribute::Bool(attr)
Expand Down
8 changes: 7 additions & 1 deletion calyx-opt/src/passes/static_promotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
}

Expand Down
22 changes: 17 additions & 5 deletions calyx-opt/src/passes/top_down_compile_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use crate::passes;
use crate::traversal::{
Action, ConstructVisitor, Named, ParseVal, PassOpt, VisResult, Visitor,
};
use calyx_ir::{self as ir, GetAttributes, LibrarySignatures, Printer, RRC};
use calyx_ir::{
self as ir, BoolAttr, GetAttributes, LibrarySignatures, Printer, RRC,
};
use calyx_ir::{build_assignments, guard, structure, Id};
use calyx_utils::Error;
use calyx_utils::{CalyxResult, OutputFile};
Expand Down Expand Up @@ -460,6 +462,8 @@ impl Schedule<'_, '_> {
preds: Vec<PredEdge>,
// 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<Vec<PredEdge>> {
match con {
// See explanation of FSM states generated in [ir::TopDownCompileControl].
Expand Down Expand Up @@ -495,7 +499,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"];
Expand Down Expand Up @@ -542,9 +546,13 @@ impl Schedule<'_, '_> {
early_transitions: bool,
) -> CalyxResult<Vec<PredEdge>> {
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)
}
Expand Down Expand Up @@ -577,6 +585,7 @@ impl Schedule<'_, '_> {
&if_stmt.tbranch,
tru_transitions,
early_transitions,
false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this unconditionally pass false instead of threading through early_transitions? Is it because we want to enable this if we see it in a seq or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only want to enable @fast on the parent control, I believe, unless you had different semantics in mind?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that makes sense! We should eventually get rid of the other early-transition logic entirely in a follow-up PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, there may be more cases in which we can apply early-transition, for example, if (as @sampsyo said) one could prove that two adjacent groups cannot be done at the same time; here, we could reuse @fast's functionality, but there might be another case where it does apply recursively. That can be dealt with later though, it's not too hard to just re-add a parameter to all the functions.

)?;
// Previous states transitioning into false branch need the conditional
// to be false.
Expand All @@ -594,6 +603,7 @@ impl Schedule<'_, '_> {
&if_stmt.fbranch,
fal_transitions,
early_transitions,
false,
)?
};

Expand Down Expand Up @@ -636,6 +646,7 @@ impl Schedule<'_, '_> {
&while_stmt.body,
transitions,
early_transitions,
false,
)?;

// Step 3: The final out edges from the while come from:
Expand Down Expand Up @@ -740,6 +751,7 @@ impl Schedule<'_, '_> {
con,
vec![first_state],
early_transitions,
false,
)?;
self.add_nxt_transition(prev);
Ok(())
Expand Down
39 changes: 39 additions & 0 deletions calyx-opt/src/passes/well_formed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use calyx_ir::{
self as ir, CellType, Component, GetAttributes, LibrarySignatures,
RESERVED_NAMES,
};
use calyx_ir::{BoolAttr, Seq};
use calyx_utils::{CalyxResult, Error, WithPos};
use ir::Nothing;
use ir::StaticTiming;
Expand Down Expand Up @@ -203,6 +204,30 @@ fn same_type(proto_out: &CellType, proto_in: &CellType) -> CalyxResult<()> {
}
}

/// 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,
Expand Down Expand Up @@ -680,6 +705,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,
Expand Down
30 changes: 27 additions & 3 deletions runt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,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 \
Expand Down Expand Up @@ -390,6 +388,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" \
ethanuppal marked this conversation as resolved.
Show resolved Hide resolved
-s verilog.data {}.data \
-s jq.expr ".memories" \
{} -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" \
-s verilog.data {}.data \
-s jq.expr ".memories" \
{} -q
"""

[[tests]]
name = "[frontend] systolic array relu static correctness"
paths = ["tests/correctness/systolic/relu-inputs/*.data"]
Expand Down
9 changes: 9 additions & 0 deletions tests/correctness/fast-attr/invalid/invalid.expect
Original file line number Diff line number Diff line change
@@ -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' failed:
=====STDERR=====
Error: Malformed Control: `seq` marked `@fast` does not contain alternating static-dynamic control children (see #1828)

=====STDOUT=====

46 changes: 46 additions & 0 deletions tests/correctness/fast-attr/invalid/invalid.futil
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
12 changes: 12 additions & 0 deletions tests/correctness/fast-attr/invalid/invalid.futil.data
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"m": {
"data": [
0
],
"format": {
"numeric_type": "bitnum",
"is_signed": false,
"width": 32
}
}
}
5 changes: 5 additions & 0 deletions tests/correctness/fast-attr/valid/fast6inc.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
ethanuppal marked this conversation as resolved.
Show resolved Hide resolved
"m": [
6
]
}
46 changes: 46 additions & 0 deletions tests/correctness/fast-attr/valid/fast6inc.futil
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
12 changes: 12 additions & 0 deletions tests/correctness/fast-attr/valid/fast6inc.futil.data
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"m": {
"data": [
0
],
"format": {
"numeric_type": "bitnum",
"is_signed": false,
"width": 32
}
}
}
Loading