WIP adding new test for L2 reg file store

crates/cpu/src/rename_execute_retire.rs

@@ -570,7 +570,7 @@ struct RobEntryDebugState<C: PhantomConstGet<CpuConfig>> {
     mop: MOpInstance<RenamedMOp<C>>,
     mop_in_unit_state: SimOnlyMOpInUnitState,
     is_speculative: Bool,
-    finished: HdlOption<NextPcPredictorOp<C>>,
+    output: HdlOption<NextPcPredictorOp<C>>,
     caused_cancel: HdlOption<UnitCausedCancel<C>>,
 }
 
@@ -581,7 +581,7 @@ impl<C: PhantomConstCpuConfig> SimValueDefault for RobEntryDebugState<C> {
             mop,
             mop_in_unit_state: _,
             is_speculative: _,
-            finished,
+            output,
             caused_cancel,
         } = self;
         #[hdl(sim)]
@@ -589,8 +589,8 @@ impl<C: PhantomConstCpuConfig> SimValueDefault for RobEntryDebugState<C> {
             mop: zeroed(mop),
             mop_in_unit_state: SimOnlyValue::default(),
             is_speculative: false,
-            finished: #[hdl(sim)]
-            finished.HdlNone(),
+            output: #[hdl(sim)]
+            output.HdlNone(),
             caused_cancel: #[hdl(sim)]
             caused_cancel.HdlNone(),
         }
@@ -602,7 +602,7 @@ struct RobEntry<C: PhantomConstCpuConfig> {
     mop: SimValue<MOpInstance<RenamedMOp<C>>>,
     mop_in_unit_state: MOpInUnitState,
     is_speculative: bool,
-    finished: Option<SimValue<NextPcPredictorOp<C>>>,
+    output: Option<SimValue<NextPcPredictorOp<C>>>,
     caused_cancel: Option<SimValue<UnitCausedCancel<C>>>,
 }
 
@@ -612,7 +612,7 @@ impl<C: PhantomConstCpuConfig> RobEntry<C> {
             mop,
             mop_in_unit_state: MOpInUnitState::NotYetEnqueued,
             is_speculative: true,
-            finished: None,
+            output: None,
             caused_cancel: None,
         }
     }
@@ -637,7 +637,7 @@ impl<C: PhantomConstCpuConfig> RobEntry<C> {
             mop,
             mop_in_unit_state,
             is_speculative,
-            finished,
+            output,
             caused_cancel,
         } = self;
         let ret_ty = RobEntryDebugState[config];
@@ -646,7 +646,7 @@ impl<C: PhantomConstCpuConfig> RobEntry<C> {
             mop,
             mop_in_unit_state: SimOnlyValue::new(*mop_in_unit_state),
             is_speculative,
-            finished: finished.into_sim_value_with_type(ret_ty.finished),
+            output: output.into_sim_value_with_type(ret_ty.output),
             caused_cancel: caused_cancel.into_sim_value_with_type(ret_ty.caused_cancel),
         }
     }
@@ -1044,8 +1044,8 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             let masked_id = rob.mop.id.as_int() as usize & mask;
             **retval[masked_id] = fmt::from_fn(|f| {
                 f.write_str(rob.mop_in_unit_state.debug_str())?;
-                if rob.finished.is_some() {
-                    f.write_str("(finished)")?;
+                if rob.output.is_some() {
+                    f.write_str("(output)")?;
                 }
                 if rob.caused_cancel.is_some() {
                     f.write_str("(caused cancel)")?;
@@ -1244,10 +1244,13 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
         &mut self,
         insn: SimValue<MOpInstance<MOp>>,
     ) -> Result<(), SimValue<MOpInstance<MOp>>> {
+        println!("try_rename: insn: {insn:?}");
         if self.rob.unrenamed_len() >= self.config.get().rob_size.get() {
+            println!("try_rename: unrenamed_len >= rob_size");
             return Err(insn);
         }
         if self.rob.renamed_len() >= self.config.get().rob_size.get() {
+            println!("try_rename: renamed_len >= rob_size");
             return Err(insn);
         }
         let unit_kind = UnitMOp::kind_sim(&insn.mop);
@@ -1323,9 +1326,11 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             );
         };
         if space_available == 0 {
+            println!("try_rename: space_available = 0");
             return Err(insn);
         }
         let Some(out_reg_num) = out_reg_num else {
+            println!("try_rename: out_reg_num = None");
             return if self.space_available_for_unit(self.l2_reg_file_unit_index) > 0
                 && let Some(l2_reg_index) = self.find_free_l2_reg()
             {
@@ -1408,6 +1413,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
                 );
                 Ok(())
             } else {
+                println!("try_rename: l2 reg file has no space and/or has no free output regs");
                 Err(insn)
             };
         }
@@ -1567,15 +1573,15 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             mop: _,
             mop_in_unit_state,
             is_speculative: _,
-            finished,
+            output,
             caused_cancel,
         } = rob;
-        assert!(finished.is_none());
+        assert!(output.is_none());
         assert!(caused_cancel.is_none());
         let l1_reg = &mut self.l1_reg_file[unit_index][out_reg_index];
         assert!(l1_reg.is_none());
         *l1_reg = Some(dest_value);
-        *finished = Some(predictor_op);
+        *output = Some(predictor_op);
         *mop_in_unit_state = mop_in_unit_state
             .after_output_ready()
             .expect("should be valid state for output to become ready");
@@ -1596,7 +1602,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             mop: _,
             mop_in_unit_state,
             is_speculative: _,
-            finished,
+            output,
             caused_cancel,
         } = self.rob.renamed_by_id_mut(&id);
         assert!(caused_cancel.is_none());
@@ -1616,7 +1622,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             panic!(
                 "MOp {id:?} made an invalid attempt to finish/cause a cancel:\n\
                     mop_in_unit_state={mop_in_unit_state:?}\n\
-                    finished={finished:?}\n\
+                    output={output:?}\n\
                     caused_cancel={caused_cancel:?}"
             );
         }
@@ -1636,10 +1642,10 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
         }
     }
     fn handle_from_post_decode(&mut self, insns: &[SimValue<MOpInstance<MOp>>]) {
-        if insns.is_empty() {
+        if self.is_canceling() {
+            assert!(insns.is_empty());
             return;
         }
-        assert!(!self.is_canceling());
         for insn in insns {
             self.rename_delayed.push_back(insn.clone());
         }
@@ -1695,7 +1701,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
                     mop: _,
                     mop_in_unit_state: MOpInUnitState::FinishedAndOrCausedCancel,
                     is_speculative: _,
-                    finished,
+                    output,
                     caused_cancel,
                 } = renamed_entry
                 {
@@ -1703,7 +1709,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
                         // only the part before the cancel needs to be ready
                         break;
                     }
-                    assert!(finished.is_some());
+                    assert!(output.is_some());
                 } else {
                     // group isn't ready
                     return retval;
@@ -1740,7 +1746,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
                         config: self.config,
                     };
                     for renamed in renamed_entries {
-                        let Some(finished) = &renamed.finished else {
+                        let Some(output) = &renamed.output else {
                             unreachable!();
                         };
                         #[hdl(sim)]
@@ -1748,7 +1754,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
                             call_stack_op,
                             cond_br_taken,
                             config: _,
-                        } = finished;
+                        } = output;
                         #[hdl(sim)]
                         if let CallStackOp::None = &unrenamed_op.call_stack_op {
                             unrenamed_op.call_stack_op = call_stack_op.clone();
@@ -1826,7 +1832,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             mop: _,
             mop_in_unit_state,
             is_speculative: _,
-            finished: _,
+            output: _,
             caused_cancel,
         } in renamed_entries
         {
@@ -1879,7 +1885,7 @@ impl<C: PhantomConstCpuConfig> RenameExecuteRetireState<C> {
             mop: _,
             mop_in_unit_state: MOpInUnitState::FinishedAndOrCausedCancel,
             is_speculative: _,
-            finished: _,
+            output: _,
             caused_cancel: Some(caused_cancel),
         }) = first_renamed
             && !*caused_cancel.cancel_after_retire
@@ -1989,10 +1995,10 @@ async fn rename_execute_retire_run(
                         mop: _,
                         mop_in_unit_state,
                         is_speculative: _,
-                        finished,
+                        output,
                         caused_cancel,
                     } = state.rob.renamed_by_id_mut(&enqueue.mop.id);
-                    assert!(finished.is_none());
+                    assert!(output.is_none());
                     assert!(caused_cancel.is_none());
                     *mop_in_unit_state = mop_in_unit_state
                         .after_enqueue()
@@ -2006,10 +2012,10 @@ async fn rename_execute_retire_run(
                     mop: _,
                     mop_in_unit_state,
                     is_speculative: _,
-                    finished,
+                    output,
                     caused_cancel,
                 } = state.rob.renamed_by_id_mut(&inputs_ready.mop.id);
-                assert!(finished.is_none());
+                assert!(output.is_none());
                 assert!(caused_cancel.is_none());
                 *mop_in_unit_state = mop_in_unit_state
                     .with_inputs_ready()
@@ -2024,11 +2030,9 @@ async fn rename_execute_retire_run(
                     mop: _,
                     mop_in_unit_state,
                     is_speculative: _,
-                    finished,
-                    caused_cancel,
+                    output: _,
+                    caused_cancel: _,
                 } = state.rob.renamed_by_id_mut(&is_no_longer_speculative.id);
-                assert!(finished.is_none());
-                assert!(caused_cancel.is_none());
                 *mop_in_unit_state = mop_in_unit_state
                     .without_speculative()
                     .expect("UnitMOpIsNoLongerSpeculative is known to be valid");
@@ -2043,10 +2047,9 @@ async fn rename_execute_retire_run(
                         mop: _,
                         mop_in_unit_state,
                         is_speculative: _,
-                        finished,
+                        output: _,
                         caused_cancel,
                     } = state.rob.renamed_by_id_mut(&id);
-                    assert!(finished.is_none());
                     assert!(caused_cancel.is_none());
                     *mop_in_unit_state = mop_in_unit_state
                         .with_cant_cause_cancel()

crates/cpu/tests/rename_execute_retire.rs

@@ -291,7 +291,34 @@ impl InsnsBuilder {
                         .cast_to_static::<SInt<_>>(),
                     true,
                     ConditionMode.SGt(),
+                    true,
+                    true,
                     false,
+                    false,
+                )]
+            },
+        ));
+    }
+    fn power_isa_beq(&mut self, target: InsnsBuilderLabel) {
+        let pc = self.pc;
+        self.add_insn(Insn::new_lazy(
+            4,
+            format!("beq {}", self.labels[target.0].name),
+            move |labels| {
+                [BranchMOp::branch_cond_ctr(
+                    MOpDestReg::new_sim(&[], &[]),
+                    [
+                        MOpRegNum::power_isa_cr_reg_imm(0),
+                        MOpRegNum::const_zero(),
+                        MOpRegNum::const_zero(),
+                    ],
+                    labels[target.0]
+                        .pc()
+                        .wrapping_sub(pc)
+                        .cast_to_static::<SInt<_>>(),
+                    false,
+                    ConditionMode.Eq(),
+                    true,
                     true,
                     false,
                     false,
@@ -1272,7 +1299,12 @@ impl MockMemory {
         src_values: &[SimValue<PRegValue>; COMMON_MOP_SRC_LEN],
         is_speculative: bool,
     ) -> Result<SimValue<PRegValue>, AddressCantBeSpeculativelyAccessed> {
-        #[hdl(sim)]
+        println!("MockMemory::run_mop: {:#x}: {:?}", mop.pc.as_int(), mop.mop);
+        println!(
+            "<- {}{src_values:?}",
+            if is_speculative { "(speculative) " } else { "" },
+        );
+        let retval = #[hdl(sim)]
         match &mop.mop {
             LoadStoreMOp::<_, _>::Load(mop) => {
                 #[hdl(sim)]
@@ -1323,13 +1355,11 @@ impl MockMemory {
                         }
                     }
                 };
-                Ok(
-                    #[hdl(sim)]
-                    PRegValue {
-                        int_fp: loaded,
-                        flags: PRegFlags::zeroed_sim(),
-                    },
-                )
+                #[hdl(sim)]
+                PRegValue {
+                    int_fp: loaded,
+                    flags: PRegFlags::zeroed_sim(),
+                }
             }
             LoadStoreMOp::<_, _>::Store(mop) => {
                 #[hdl(sim)]
@@ -1363,9 +1393,11 @@ impl MockMemory {
                         }
                     }
                 }
-                Ok(PRegValue::zeroed_sim())
+                PRegValue::zeroed_sim()
             }
-        }
+        };
+        println!("-> {retval:?}");
+        Ok(retval)
     }
 }
 
@@ -2977,3 +3009,135 @@ fn test_rename_execute_retire_fibonacci() {
         panic!();
     }
 }
+
+struct SlowLoopInsns;
+
+impl SlowLoopInsns {
+    const CONSTANTS_ADDR: u64 = 0x4000;
+    const CONSTANTS_COUNT: usize = 5;
+    const CONSTANTS_STEP: usize = 8;
+    const LOG2_RESULT_FACTOR: u32 = 2;
+}
+
+impl MakeInsns for SlowLoopInsns {
+    fn make_insns() -> Insns {
+        let mut b = InsnsBuilder::new();
+
+        let slow_loop = b.new_label("slow_loop");
+        b.power_isa_ld(3, 0, MockMemory::IO_ADDR as i16); // load input
+        b.power_isa_addi(1, 0, 0x4000); // setup stack pointer
+        b.power_isa_bl(slow_loop);
+        b.power_isa_std(3, 0, MockMemory::IO_ADDR as i16); // store output
+        let done = b.new_defined_label("done");
+        b.power_isa_b(done);
+
+        b.set_pc(0x1000);
+        b.define_label(slow_loop);
+
+        let loop_header = b.new_defined_label("loop_header");
+        b.power_isa_addi(4, 0, 0); // clear sum
+        b.power_isa_cmpldi(0, 3, 0);
+        let loop_done = b.new_label("loop_done");
+        b.power_isa_beq(loop_done); // if input == 0 goto loop_done
+
+        // long sequence of loads to provoke L2 register file store
+        let start_reg = 5;
+        assert!(
+            start_reg + Self::CONSTANTS_COUNT <= 32,
+            "too many constants to load them all into PowerISA GPRs",
+        );
+        for i in 0..Self::CONSTANTS_COUNT {
+            b.power_isa_ld(
+                start_reg + i,
+                0,
+                (Self::CONSTANTS_ADDR + (Self::CONSTANTS_STEP * i) as u64) as i16,
+            );
+        }
+        for i in 1..Self::CONSTANTS_COUNT {
+            b.power_isa_add(4, 4, start_reg + i);
+        }
+
+        b.power_isa_addi(3, 3, -1);
+        b.power_isa_b(loop_header);
+
+        b.define_label(loop_done);
+        b.power_isa_mr(3, 4);
+        for _ in 0..Self::LOG2_RESULT_FACTOR {
+            b.power_isa_add(3, 3, 3);
+        }
+        b.power_isa_blr(); // return
+
+        b.build()
+    }
+    fn make_load_store_execution_state() -> MockMemory {
+        let expected = 0x0123_4567_89AB_CDEF_u64;
+        let constants: [[u8; Self::CONSTANTS_STEP]; Self::CONSTANTS_COUNT] =
+            std::array::from_fn(|i| {
+                let start_bit_index = i * 64 / Self::CONSTANTS_COUNT;
+                let end_bit_index = (i + 1) * 64 / Self::CONSTANTS_COUNT;
+                let start_bit = 1u64.unbounded_shl(start_bit_index as u32);
+                let end_bit = 1u64.unbounded_shl(end_bit_index as u32);
+                let mask = end_bit.wrapping_sub(start_bit);
+                (expected & mask).to_le_bytes()
+            });
+        let loop_count = 4;
+        MockMemory::new(
+            loop_count,
+            expected * loop_count * 2u64.pow(Self::LOG2_RESULT_FACTOR),
+            [(Self::CONSTANTS_ADDR, constants.as_flattened())],
+        )
+    }
+}
+
+#[hdl]
+#[test]
+fn test_rename_execute_retire_slow_loop() {
+    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::AluBranch),
+            UnitConfig::new(UnitKind::LoadStore),
+            UnitConfig::new(UnitKind::TransformedMove),
+        ],
+        NonZeroUsize::new(20).unwrap(),
+    );
+    config.fetch_width = NonZeroUsize::new(4).unwrap();
+    let m = rename_execute_retire_test_harness::<SlowLoopInsns>(PhantomConst::new_sized(config));
+    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_slow_loop.vcd") {
+        panic!();
+    }
+}