tests/rename_execute_retire: add and use mock_combinational_unit

crates/cpu/src/rename_execute_retire.rs

@@ -196,7 +196,9 @@ pub struct UnitMOpCantCauseCancel<C: PhantomConstGet<CpuConfig>> {
 pub struct ExecuteToUnitInterface<C: PhantomConstGet<CpuConfig>> {
     /// Enqueues happen in program order, they are not re-ordered by out-of-order execution.
     pub enqueue: ReadyValid<UnitEnqueue<C>>,
+    /// if [`Self::unit_outputs_ready`] is `false`, then this is always [`HdlNone`]
     pub inputs_ready: HdlOption<UnitInputsReady<C>>,
+    /// if [`Self::unit_outputs_ready`] is `false`, then this is always [`HdlNone`]
     pub is_no_longer_speculative: HdlOption<UnitMOpIsNoLongerSpeculative<C>>,
     /// this uses [`Self::unit_outputs_ready`] as a shared ready flag
     #[hdl(flip)]

crates/cpu/tests/rename_execute_retire.rs

@@ -2479,24 +2479,6 @@ impl<C: PhantomConstCpuConfig, E: MockExecutionStateTrait> MockUnitState<C, E> {
     }
 }
 
-fn is_the_l2_reg_file_unit(config: PhantomConst<CpuConfig>, unit_index: usize) -> bool {
-    let first_unit_index = config
-        .get()
-        .units
-        .iter()
-        .position(|v| v.kind == UnitKind::TransformedMove);
-    let last_unit_index = config
-        .get()
-        .units
-        .iter()
-        .rposition(|v| v.kind == UnitKind::TransformedMove);
-    assert_eq!(
-        first_unit_index, last_unit_index,
-        "multiple L2 reg file units aren't allowed"
-    );
-    Some(unit_index) == first_unit_index
-}
-
 #[hdl_module(extern)]
 fn mock_unit<#[hdl(skip)] E: MockExecutionStateTrait>(
     config: PhantomConst<CpuConfig>,
@@ -2671,6 +2653,95 @@ fn mock_unit<#[hdl(skip)] E: MockExecutionStateTrait>(
     }
 }
 
+#[hdl_module(extern)]
+fn mock_combinational_unit<#[hdl(skip)] E: MockExecutionStateTrait>(
+    config: PhantomConst<CpuConfig>,
+    unit_index: usize,
+) {
+    #[hdl]
+    let from_execute: ExecuteToUnitInterface<PhantomConst<CpuConfig>> =
+        m.input(ExecuteToUnitInterface[config]);
+    m.extern_module_simulation_fn((from_execute, config, unit_index), async |args, mut sim| {
+        let (from_execute, config, unit_index) = args;
+        #[hdl]
+        let ExecuteToUnitInterface::<_> {
+            enqueue,
+            inputs_ready: _,
+            is_no_longer_speculative: _,
+            cant_cause_cancel,
+            output_ready,
+            finish_cause_cancel,
+            unit_outputs_ready: _,
+            cancel_all,
+            config: _,
+        } = from_execute;
+        // initialize all outputs
+        sim.write(enqueue.ready, false).await;
+        sim.write(cancel_all.ready, false).await;
+        sim.write(
+            cant_cause_cancel,
+            #[hdl(sim)]
+            (cant_cause_cancel.ty()).HdlNone(),
+        )
+        .await;
+        sim.write(
+            output_ready,
+            #[hdl(sim)]
+            (output_ready.ty()).HdlNone(),
+        )
+        .await;
+        sim.write(
+            finish_cause_cancel,
+            #[hdl(sim)]
+            (finish_cause_cancel.ty()).HdlNone(),
+        )
+        .await;
+        run_fn::<E>(from_execute, config, unit_index, sim).await;
+    });
+    #[hdl]
+    async fn run_fn<E: MockExecutionStateTrait>(
+        from_execute: Expr<ExecuteToUnitInterface<PhantomConst<CpuConfig>>>,
+        config: PhantomConst<CpuConfig>,
+        unit_index: usize,
+        mut sim: ExternModuleSimulationState,
+    ) {
+        loop {
+            #[hdl]
+            let ExecuteToUnitInterface::<_> {
+                enqueue,
+                inputs_ready,
+                is_no_longer_speculative: _, // we don't care about being speculative for these instructions
+                cant_cause_cancel,
+                output_ready,
+                finish_cause_cancel,
+                unit_outputs_ready: _,
+                cancel_all,
+                config: _,
+            } = from_execute;
+            sim.write(enqueue.ready, true).await; // we ignore enqueues since we don't need to track order for these instructions
+            let mut state = MockUnitState::new(E::default(), config, unit_index);
+            #[hdl(sim)]
+            if let HdlSome(inputs_ready) = sim.read(inputs_ready).await {
+                state.handle_inputs_ready(inputs_ready);
+            }
+            // all outputs are immediately ready, so reporting that instructions can't cause cancels is superfluous
+            sim.write(
+                cant_cause_cancel,
+                #[hdl(sim)]
+                (cant_cause_cancel.ty()).HdlNone(),
+            )
+            .await;
+            let (output_ready_v, finish_cause_cancel_v) =
+                state.peek_output_ready_and_finish_cause_cancel();
+            sim.write(output_ready, output_ready_v).await;
+            sim.write(finish_cause_cancel, finish_cause_cancel_v).await;
+            sim.write(cancel_all.ready, true).await; // this unit is purely combinational so canceling does nothing
+            sim.wait_for_changes([Expr::canonical(inputs_ready)], None)
+                .await;
+        }
+    }
+}
+
 #[hdl(no_static)]
 struct MockL2RegFileOpDebugState<C: PhantomConstGet<CpuConfig>> {
     mop: MOpInstance<L2RegisterFileMOp<PRegNum<C>, PRegNum<C>>>,
@@ -3913,7 +3984,10 @@ fn mock_load_store_unit<#[hdl(skip)] MI: MakeInsns>(
 }
 
 #[hdl_module]
-fn rename_execute_retire_test_harness<#[hdl(skip)] MI: MakeInsns>(config: PhantomConst<CpuConfig>) {
+fn rename_execute_retire_test_harness<#[hdl(skip)] MI: MakeInsns>(
+    config: PhantomConst<CpuConfig>,
+    alu_branch_is_combinatorial: bool,
+) {
     #[hdl]
     let cd: ClockDomain = m.input();
     #[hdl]
@@ -3959,13 +4033,22 @@ fn rename_execute_retire_test_harness<#[hdl(skip)] MI: MakeInsns>(config: Phanto
                 connect(started_any_l2_reg_file_ops, mock_unit.started_any);
             }
             UnitKind::AluBranch => {
-                let mock_unit = instance_with_loc(
-                    &dut.ty().to_units.unit_field_name(unit_index),
-                    mock_unit::<()>(config, unit_index),
-                    SourceLocation::caller(),
-                );
-                connect(mock_unit.cd, cd);
-                connect(mock_unit.from_execute, to_unit);
+                if alu_branch_is_combinatorial {
+                    let mock_unit = instance_with_loc(
+                        &dut.ty().to_units.unit_field_name(unit_index),
+                        mock_combinational_unit::<()>(config, unit_index),
+                        SourceLocation::caller(),
+                    );
+                    connect(mock_unit.from_execute, to_unit);
+                } else {
+                    let mock_unit = instance_with_loc(
+                        &dut.ty().to_units.unit_field_name(unit_index),
+                        mock_unit::<()>(config, unit_index),
+                        SourceLocation::caller(),
+                    );
+                    connect(mock_unit.cd, cd);
+                    connect(mock_unit.from_execute, to_unit);
+                }
             }
         }
     }
@@ -4026,7 +4109,7 @@ impl MakeInsns for FibonacciInsns {
 
 #[hdl]
 #[test]
-fn test_rename_execute_retire_fibonacci() {
+fn test_rename_execute_retire_fibonacci_non_combinatorial() {
     let _n = SourceLocation::normalize_files_for_tests();
     let mut config = CpuConfig::new(
         vec![
@@ -4039,7 +4122,10 @@ fn test_rename_execute_retire_fibonacci() {
         NonZeroUsize::new(20).unwrap(),
     );
     config.fetch_width = NonZeroUsize::new(3).unwrap();
-    let m = rename_execute_retire_test_harness::<FibonacciInsns>(PhantomConst::new_sized(config));
+    let m = rename_execute_retire_test_harness::<FibonacciInsns>(
+        PhantomConst::new_sized(config),
+        false,
+    );
     let mut sim = Simulation::new(m);
     let writer = RcWriter::default();
     sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
@@ -4071,7 +4157,60 @@ fn test_rename_execute_retire_fibonacci() {
     // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
     let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
     println!("####### VCD:\n{vcd}\n#######");
-    if vcd != include_str!("expected/rename_execute_retire_fibonacci.vcd") {
+    if vcd != include_str!("expected/rename_execute_retire_fibonacci_non_combinatorial.vcd") {
+        panic!();
+    }
+}
+
+#[hdl]
+#[test]
+fn test_rename_execute_retire_fibonacci_combinatorial() {
+    let _n = SourceLocation::normalize_files_for_tests();
+    let mut config = CpuConfig::new(
+        vec![
+            UnitConfig::new(UnitKind::AluBranch),
+            UnitConfig::new(UnitKind::AluBranch),
+            UnitConfig::new(UnitKind::AluBranch),
+            UnitConfig::new(UnitKind::LoadStore),
+            UnitConfig::new(UnitKind::TransformedMove),
+        ],
+        NonZeroUsize::new(20).unwrap(),
+    );
+    config.fetch_width = NonZeroUsize::new(3).unwrap();
+    let m =
+        rename_execute_retire_test_harness::<FibonacciInsns>(PhantomConst::new_sized(config), true);
+    let mut sim = Simulation::new(m);
+    let writer = RcWriter::default();
+    sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+    struct DumpVcdOnDrop {
+        writer: Option<RcWriter>,
+    }
+    impl Drop for DumpVcdOnDrop {
+        fn drop(&mut self) {
+            if let Some(mut writer) = self.writer.take() {
+                let vcd = String::from_utf8(writer.take()).unwrap();
+                println!("####### VCD:\n{vcd}\n#######");
+            }
+        }
+    }
+    let mut writer = DumpVcdOnDrop {
+        writer: Some(writer),
+    };
+    sim.write_clock(sim.io().cd.clk, false);
+    sim.write_reset(sim.io().cd.rst, true);
+    for cycle in 0..200 {
+        sim.advance_time(SimDuration::from_nanos(500));
+        println!("clock tick: {cycle}");
+        sim.write_clock(sim.io().cd.clk, true);
+        sim.advance_time(SimDuration::from_nanos(500));
+        sim.write_clock(sim.io().cd.clk, false);
+        sim.write_reset(sim.io().cd.rst, false);
+    }
+    assert!(sim.read_bool(sim.io().all_outputs_written));
+    // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
+    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
+    println!("####### VCD:\n{vcd}\n#######");
+    if vcd != include_str!("expected/rename_execute_retire_fibonacci_combinatorial.vcd") {
         panic!();
     }
 }
@@ -4171,7 +4310,8 @@ fn test_rename_execute_retire_slow_loop() {
         NonZeroUsize::new(20).unwrap(),
     );
     config.fetch_width = NonZeroUsize::new(4).unwrap();
-    let m = rename_execute_retire_test_harness::<SlowLoopInsns>(PhantomConst::new_sized(config));
+    let m =
+        rename_execute_retire_test_harness::<SlowLoopInsns>(PhantomConst::new_sized(config), true);
     let mut sim = Simulation::new(m);
     let writer = RcWriter::default();
     sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
@@ -4309,7 +4449,8 @@ fn test_rename_execute_retire_head_n1() {
         NonZeroUsize::new(20).unwrap(),
     );
     config.fetch_width = NonZeroUsize::new(2).unwrap();
-    let m = rename_execute_retire_test_harness::<HeadN1Insns>(PhantomConst::new_sized(config));
+    let m =
+        rename_execute_retire_test_harness::<HeadN1Insns>(PhantomConst::new_sized(config), true);
     let mut sim = Simulation::new(m);
     let writer = RcWriter::default();
     sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));