From a21f7e0ab49dce6bbd18b4e80a6939142d7b7734 Mon Sep 17 00:00:00 2001 From: Ayaka Yorihiro <36107281+ayakayorihiro@users.noreply.github.com> Date: Wed, 23 Oct 2024 12:00:33 -0400 Subject: [PATCH] [Profiler] Pass to produce instrumentation code (#2310) This PR contains (in order of importance): - `instrument.rs`: a new pass (`-p instrument`) that adds instrumentation to each group. - A new primitive (`std_inst_wire`) which functionally works the same as `std_wire`, but specifically is *not* a combinational component. This is important for preventing optimizations from removing any cells of said primitive. - `compare-instrumentation.sh`: a script to make sure that instrumentation did not affect latency. - Changes to test expect files to include the new `std_inst_wire` primitive. ## Instrumentation The `instrument` pass iterates over all groups. For a group `foo`, it produces (1) `foo_inst`, a `std_inst_wire` cell, and (2) an assignment within `foo` that sets `foo_inst` to 1. This makes `foo_inst` have 1 whenever the group `foo` is active. --- .gitignore | 1 + calyx-opt/src/default_passes.rs | 11 +-- calyx-opt/src/passes/mod.rs | 2 + .../src/passes/profiler_instrumentation.rs | 72 +++++++++++++++++++ tests/passes/profiler_instrumentation.expect | 46 ++++++++++++ tests/passes/profiler_instrumentation.futil | 48 +++++++++++++ tools/profiler/compare-instrumentation.sh | 41 +++++++++++ 7 files changed, 217 insertions(+), 4 deletions(-) create mode 100644 calyx-opt/src/passes/profiler_instrumentation.rs create mode 100644 tests/passes/profiler_instrumentation.expect create mode 100644 tests/passes/profiler_instrumentation.futil create mode 100644 tools/profiler/compare-instrumentation.sh diff --git a/.gitignore b/.gitignore index 29eaadf2e..5c6840dac 100644 --- a/.gitignore +++ b/.gitignore @@ -62,6 +62,7 @@ tools/profiler/data tools/profiler/meta-logs tools/profiler/fg-tmp tools/profiler/handmade-flame-graphs/*/*.svg +tools/profiler/inst-check-tmp temp/ diff --git a/calyx-opt/src/default_passes.rs b/calyx-opt/src/default_passes.rs index 75e1e7adf..1280eada9 100644 --- a/calyx-opt/src/default_passes.rs +++ b/calyx-opt/src/default_passes.rs @@ -5,10 +5,11 @@ use crate::passes::{ CompileInvoke, CompileRepeat, CompileStatic, ComponentInliner, DataPathInfer, DeadAssignmentRemoval, DeadCellRemoval, DeadGroupRemoval, DefaultAssigns, Externalize, GoInsertion, GroupToInvoke, GroupToSeq, - InferShare, LowerGuards, MergeAssign, Papercut, RemoveIds, ResetInsertion, - SimplifyStaticGuards, SimplifyWithControl, StaticFSMOpts, StaticInference, - StaticInliner, StaticPromotion, SynthesisPapercut, TopDownCompileControl, - UnrollBounded, WellFormed, WireInliner, WrapMain, + InferShare, LowerGuards, MergeAssign, Papercut, ProfilerInstrumentation, + RemoveIds, ResetInsertion, SimplifyStaticGuards, SimplifyWithControl, + StaticFSMOpts, StaticInference, StaticInliner, StaticPromotion, + SynthesisPapercut, TopDownCompileControl, UnrollBounded, WellFormed, + WireInliner, WrapMain, }; use crate::passes_experimental::{ CompileSync, CompileSyncWithoutSyncReg, DiscoverExternal, ExternalToRef, @@ -78,6 +79,8 @@ impl PassManager { pm.register_pass::()?; pm.register_pass::()?; + pm.register_pass::()?; + //add metadata pm.register_pass::()?; diff --git a/calyx-opt/src/passes/mod.rs b/calyx-opt/src/passes/mod.rs index e71fe9260..742e6f7ef 100644 --- a/calyx-opt/src/passes/mod.rs +++ b/calyx-opt/src/passes/mod.rs @@ -20,6 +20,7 @@ mod lower_guards; pub mod math_utilities; mod merge_assign; mod papercut; +mod profiler_instrumentation; mod reset_insertion; mod simplify_static_guards; mod static_fsm_opts; @@ -63,6 +64,7 @@ pub use lower_guards::LowerGuards; pub use math_utilities::get_bit_width_from; pub use merge_assign::MergeAssign; pub use papercut::Papercut; +pub use profiler_instrumentation::ProfilerInstrumentation; pub use remove_ids::RemoveIds; pub use reset_insertion::ResetInsertion; pub use simplify_static_guards::SimplifyStaticGuards; diff --git a/calyx-opt/src/passes/profiler_instrumentation.rs b/calyx-opt/src/passes/profiler_instrumentation.rs new file mode 100644 index 000000000..829861ec9 --- /dev/null +++ b/calyx-opt/src/passes/profiler_instrumentation.rs @@ -0,0 +1,72 @@ +use crate::traversal::{Action, ConstructVisitor, Named, VisResult, Visitor}; +use calyx_ir::{self as ir, build_assignments, BoolAttr}; +use calyx_utils::CalyxResult; + +/// Adds probe wires to each group to detect when a group is active. +/// Used by the profiler. +pub struct ProfilerInstrumentation {} + +impl Named for ProfilerInstrumentation { + fn name() -> &'static str { + "profiler-instrumentation" + } + + fn description() -> &'static str { + "Add instrumentation for profiling" + } + + fn opts() -> Vec { + vec![] + } +} + +impl ConstructVisitor for ProfilerInstrumentation { + fn from(_ctx: &ir::Context) -> CalyxResult + where + Self: Sized + Named, + { + Ok(ProfilerInstrumentation {}) + } + + fn clear_data(&mut self) {} +} + +impl Visitor for ProfilerInstrumentation { + fn start( + &mut self, + comp: &mut ir::Component, + sigs: &ir::LibrarySignatures, + _comps: &[ir::Component], + ) -> VisResult { + // collect names of all groups (to construct group-specific cells) + let group_names = comp + .groups + .iter() + .map(|group| group.borrow().name()) + .collect::>(); + // for each group, construct a instrumentation cell and instrumentation assignment + let mut asgn_and_cell = Vec::with_capacity(group_names.len()); + { + let mut builder = ir::Builder::new(comp, sigs); + let one = builder.add_constant(1, 1); + for group_name in group_names.into_iter() { + let name = format!("{}_probe", group_name); + let inst_cell = builder.add_primitive(name, "std_wire", &[1]); + let asgn: [ir::Assignment; 1] = build_assignments!( + builder; + inst_cell["in"] = ? one["out"]; + ); + inst_cell.borrow_mut().add_attribute(BoolAttr::Protected, 1); + asgn_and_cell.push((asgn[0].clone(), inst_cell)); + } + } + // add cells and assignments + for (group, (asgn, inst_cell)) in + comp.groups.iter().zip(asgn_and_cell.into_iter()) + { + group.borrow_mut().assignments.push(asgn); + comp.cells.add(inst_cell); + } + Ok(Action::Stop) + } +} diff --git a/tests/passes/profiler_instrumentation.expect b/tests/passes/profiler_instrumentation.expect new file mode 100644 index 000000000..ec7b21475 --- /dev/null +++ b/tests/passes/profiler_instrumentation.expect @@ -0,0 +1,46 @@ +import "primitives/core.futil"; +import "primitives/memories/comb.futil"; +component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (@done done: 1) { + cells { + @external i = comb_mem_d1(32, 1, 1); + lt = std_lt(32); + lt_reg = std_reg(1); + add = std_add(32); + @generated @protected cond_probe = std_wire(1); + @generated @protected incr_probe = std_wire(1); + } + wires { + group cond { + i.addr0 = 1'd0; + lt.left = i.read_data; + lt.right = 32'd8; + lt_reg.in = lt.out; + lt_reg.write_en = 1'd1; + cond[done] = lt_reg.done; + cond_probe.in = 1'd1; + } + group incr<"static"=1> { + add.right = i.read_data; + add.left = 32'd1; + i.write_data = add.out; + i.addr0 = 1'd0; + i.write_en = 1'd1; + incr[done] = i.done; + incr_probe.in = 1'd1; + } + } + control { + seq { + cond; + while lt_reg.out { + seq { + incr; + incr; + cond; + } + } + } + } +} +---STDERR--- +[WARN calyx_frontend::attribute] The attribute @static is deprecated and will be ignored by the compiler. diff --git a/tests/passes/profiler_instrumentation.futil b/tests/passes/profiler_instrumentation.futil new file mode 100644 index 000000000..5ed5727da --- /dev/null +++ b/tests/passes/profiler_instrumentation.futil @@ -0,0 +1,48 @@ +//-p profiler-instrumentation + +import "primitives/core.futil"; +import "primitives/memories/comb.futil"; + +component main() -> () { + cells { + @external(1) i = comb_mem_d1(32, 1, 1); + lt = std_lt(32); + lt_reg = std_reg(1); + add = std_add(32); + } + + wires { + group cond { + i.addr0 = 1'd0; + lt.left = i.read_data; + lt.right = 32'd8; + lt_reg.in = lt.out; + lt_reg.write_en = 1'b1; + cond[done] = lt_reg.done; + } + + group incr<"static"=1> { + add.right = i.read_data; + add.left = 32'd1; + + i.write_data = add.out; + i.addr0 = 1'd0; + i.write_en = 1'b1; + + incr[done] = i.done; + } + } + + control { + seq { + cond; + while lt_reg.out { + seq { + incr; + incr; + cond; + } + } + } + } +} diff --git a/tools/profiler/compare-instrumentation.sh b/tools/profiler/compare-instrumentation.sh new file mode 100644 index 000000000..86e63b42e --- /dev/null +++ b/tools/profiler/compare-instrumentation.sh @@ -0,0 +1,41 @@ +if [ $# -lt 2 ]; then + echo "USAGE: bash $0 INPUT_FILE SIM_DATA_JSON" + exit +fi + +INPUT_FILE=$1 +SIM_DATA_JSON=$2 + +SCRIPT_DIR=$( cd $( dirname $0 ) && pwd ) +SCRIPT_NAME=$( echo "$0" | rev | cut -d/ -f1 | rev ) +CALYX_DIR=$( dirname $( dirname ${SCRIPT_DIR} ) ) +TMP_DIR=${SCRIPT_DIR}/inst-check-tmp +mkdir -p ${TMP_DIR} +rm -rf ${TMP_DIR}/* + +INST_DIR=${TMP_DIR}/inst +BASE_DIR=${TMP_DIR}/no-inst +mkdir -p ${INST_DIR} ${BASE_DIR} + +INST_FILE=${TMP_DIR}/inst.json +BASE_FILE=${TMP_DIR}/no-inst.json + +# run with instrumentation +( + cd ${CALYX_DIR} + set -o xtrace + fud2 ${INPUT_FILE} -o ${INST_FILE} --through verilator -s calyx.args='-p profiler-instrumentation -p all' -s sim.data=${SIM_DATA_JSON} --dir ${INST_DIR} + set +o xtrace +) &> ${TMP_DIR}/gol-inst + +# run without instrumentation +( + cd ${CALYX_DIR} + set -o xtrace + fud2 ${INPUT_FILE} -o ${BASE_FILE} --through verilator -s calyx.args='-p all' -s sim.data=${SIM_DATA_JSON} --dir ${BASE_DIR} + set +o xtrace +) &> ${TMP_DIR}/gol-no-inst + +# diff the two dat files +echo "Diff... should be empty" +diff ${INST_FILE} ${BASE_FILE}