fix(pipeline): build the out-of-order engine wherever a mode is set

PipelineConfig::apply_to_state assigned the mode field directly, so a
config asking for Scoreboard or Tomasulo + ROB - `raven run --pipeline
--config`, or a config imported in the TUI - got no engine, and the
in-order datapath ran in its place without a word. Copying the config to
the other harts did the same. All three go through set_mode now, and
set_mode builds the engine even when the field was already assigned.

reset_stages and redirect_pc touched only the in-order latches, so under
a dynamic model a reload, a reset or a PC edit left the engine fetching
where the old program was, with its old buffer and statistics. Both now
reach the engine, and a reset also applies the unit counts configured
since it was built. Enabling or disabling the pipeline reaches it too.

With the models really running, riscv-tests passes 71/71 under Tomasulo
+ ROB and 69/71 under Scoreboard; the two failures (rv32uf fcvt, ldst)
come from #86 and from flw/fsw not being ordered as memory accesses.

Author: Gaok1

crates/raven-engine/src/architectures/riscv32/falcon/pipeline/mod.rs

@@ -725,6 +725,9 @@ impl crate::falcon::machine::JournaledPipeline for PipelineSimState {
 impl crate::capability::PipelineControl for PipelineSimState {
     fn set_enabled(&mut self, enabled: bool) {
         self.enabled = enabled;
+        if let Some(engine) = &mut self.ooo {
+            crate::capability::PipelineControl::set_enabled(engine, enabled);
+        }
     }
 
     /// Start a fresh run from `address`: stages, units, counters and history
@@ -782,7 +785,13 @@ impl PipelineSimState {
         self.exec_regions.extend_from_slice(regions);
     }
 
+    /// Follow a jump the host made. Under a dynamically scheduled model the
+    /// engine owns the front end, so it is told too; otherwise it would go on
+    /// fetching where it was.
     pub fn redirect_pc(&mut self, new_pc: u32) {
+        if let Some(engine) = &mut self.ooo {
+            crate::capability::PipelineControl::redirect(engine, u64::from(new_pc));
+        }
         self.stages = Default::default();
         self.fu_bank = std::array::from_fn(|_| Vec::new());
         self.fu_busy = [0; 7];
@@ -805,7 +814,18 @@ impl PipelineSimState {
         self.class_counts = [0; InstrClass::COUNT];
     }
 
+    /// Start a fresh run from `base_pc`.
+    ///
+    /// A dynamically scheduled engine starts over too — with the unit counts
+    /// configured now, which may have changed since it was built — or it would
+    /// keep the old program's front end, buffer and statistics.
     pub fn reset_stages(&mut self, base_pc: u32) {
+        if let Some(engine) = &mut self.ooo {
+            for (unit, instances) in self.fu_capacity.iter().enumerate() {
+                engine.set_capacity(unit, (*instances).max(1) as usize);
+            }
+            crate::capability::PipelineControl::reset(engine, u64::from(base_pc));
+        }
         self.fetch_pc = base_pc;
         self.stages = Default::default();
         self.fu_bank = std::array::from_fn(|_| Vec::new());
@@ -844,8 +864,12 @@ impl PipelineSimState {
     /// nothing had executed — an instruction changes the machine at commit and
     /// nowhere else — so refetching from the current PC loses nothing, and the
     /// counters start over because they describe a model.
+    ///
+    /// This is the only way to change [`Self::mode`]: assigning the field
+    /// directly leaves a dynamic mode with no engine, and the in-order
+    /// datapath quietly runs in its place.
     pub fn set_mode(&mut self, mode: PipelineMode, timing: &PipelineTiming) {
-        if self.mode == mode {
+        if self.mode == mode && self.ooo.is_some() == mode.is_out_of_order() {
             return;
         }
         self.mode = mode;

src/cli/mod.rs

@@ -1032,7 +1032,7 @@ fn run_headless_pipeline(
     capture_trace: bool,
 ) -> (Result<PipelineReport, String>, Vec<PipelineTraceStep>) {
     let mut state = raven_engine::falcon::pipeline::PipelineSimState::new();
-    pcfg.apply_to_state(&mut state);
+    pcfg.apply_to_state(&mut state, cpi);
     state.reset_stages(cpu.pc);
     let mut trace_steps = Vec::new();
 

src/ui/app/runtime.rs

@@ -4,15 +4,17 @@ impl App {
     fn copy_pipeline_config_to_hart(
         src: &raven_engine::falcon::pipeline::PipelineSimState,
         dst: &mut raven_engine::falcon::pipeline::PipelineSimState,
+        timing: &CpiConfig,
     ) {
         dst.enabled = src.enabled;
         dst.sequential_mode = src.sequential_mode;
         dst.bypass = src.bypass;
         dst.branch_resolve = src.branch_resolve;
-        dst.mode = src.mode;
+        dst.fu_capacity = src.fu_capacity;
+        // Through `set_mode`, so a dynamically scheduled model gets its engine.
+        dst.set_mode(src.mode, timing);
         dst.set_predict(src.predict);
         dst.exec_regions = src.exec_regions.clone();
-        dst.fu_capacity = src.fu_capacity;
     }
 
     fn generic_pipeline_snapshot(&self, scope: &str) -> Option<PipelineResultsSnapshot> {
@@ -383,7 +385,7 @@ impl App {
             if let Some(p) = hart.pipeline.as_mut()
                 && let Some(model) = rv32_runtime(&*self.machine)
             {
-                Self::copy_pipeline_config_to_hart(model.pipeline(), p);
+                Self::copy_pipeline_config_to_hart(model.pipeline(), p, &self.session.cpi_config);
                 p.reset_stages(hart.cpu.pc);
             }
         }
@@ -447,7 +449,7 @@ impl App {
             } else if let Some(p) = runtime.pipeline.as_mut()
                 && let Some(model) = self.rv32()
             {
-                Self::copy_pipeline_config_to_hart(model.pipeline(), p);
+                Self::copy_pipeline_config_to_hart(model.pipeline(), p, &self.session.cpi_config);
                 p.reset_stages(runtime.cpu.pc);
             }
             self.harts.push(runtime);
@@ -511,7 +513,11 @@ impl App {
                 .take()
                 .unwrap_or_else(raven_engine::falcon::pipeline::PipelineSimState::new);
             if let Some(model) = self.rv32() {
-                Self::copy_pipeline_config_to_hart(model.pipeline(), &mut pipeline);
+                Self::copy_pipeline_config_to_hart(
+                    model.pipeline(),
+                    &mut pipeline,
+                    &self.session.cpi_config,
+                );
             }
             if pipeline.fetch_pc == 0 && pipeline.cycle_count == 0 {
                 pipeline.reset_stages(self.program_counter() as u32);
@@ -651,7 +657,11 @@ impl App {
         child.cpu.heap_break = self.rv32().map_or(0, |rv32| rv32.cpu().heap_break);
         child.prev_pc = child.cpu.pc;
         if let Some(p) = child.pipeline.as_mut() {
-            Self::copy_pipeline_config_to_hart(&self.native().pipeline(), p);
+            Self::copy_pipeline_config_to_hart(
+                &self.native().pipeline(),
+                p,
+                &self.session.cpi_config,
+            );
             p.reset_stages(child.cpu.pc);
         }
 
@@ -742,7 +752,11 @@ impl App {
         child.cpu.heap_break = self.harts[core_idx].cpu.heap_break;
         child.prev_pc = child.cpu.pc;
         if let Some(p) = child.pipeline.as_mut() {
-            Self::copy_pipeline_config_to_hart(&self.native().pipeline(), p);
+            Self::copy_pipeline_config_to_hart(
+                &self.native().pipeline(),
+                p,
+                &self.session.cpi_config,
+            );
             p.reset_stages(child.cpu.pc);
         }
 

src/ui/input/keyboard/serialization.rs

@@ -298,8 +298,9 @@ fn apply_config_v3(app: &mut App, cfg: ConfigV3) {
             .tlb
             .reconfigure(cfg.tlb);
     }
+    let timing = app.session.cpi_config.clone();
     if let Some(pipeline) = app.pipeline_config_mut() {
-        cfg.pipeline.apply_to_state(pipeline);
+        cfg.pipeline.apply_to_state(pipeline, &timing);
     }
     app.run.pipeline_view_mut().speed = cfg.pipeline.speed;
     // Sim settings applied last: may trigger a simulation restart.

src/ui/pipeline/mod.rs

@@ -142,12 +142,15 @@ impl PipelineConfig {
         }
     }
 
-    pub fn apply_to_state(self, state: &mut PipelineSimState) {
+    /// Configure `state` as this config says. The mode goes through
+    /// [`PipelineSimState::set_mode`], which builds the engine a dynamically
+    /// scheduled model needs — with the unit counts set just before it.
+    pub fn apply_to_state(self, state: &mut PipelineSimState, timing: &PipelineTiming) {
         state.enabled = self.enabled;
         state.bypass = self.bypass;
         state.branch_resolve = self.branch_resolve;
-        state.mode = self.mode;
         state.fu_capacity = self.fu_capacity;
+        state.set_mode(self.mode, timing);
         state.set_predict(self.predict);
     }
 }

tests/support/ui_pipeline_mod.rs

@@ -1,6 +1,7 @@
 use super::{
     BranchPredict, BranchResolve, GanttCell, GanttRow, InstrClass, PipelineBypassConfig,
-    PipelineConfig, PipelineMode, PipelineSimState, PipelineSpeed, Stage, gantt_max_scroll,
+    PipelineConfig, PipelineMode, PipelineSimState, PipelineSpeed, PipelineTiming, Stage,
+    gantt_max_scroll,
     gantt_view_rows, gantt_window_bounds, parse_pipeline_config, serialize_pipeline_config,
 };
 use std::collections::VecDeque;
@@ -198,3 +199,23 @@ fn gantt_max_scroll_matches_visible_history_capacity() {
 
     assert_eq!(gantt_max_scroll(&state, 7), 7);
 }
+
+#[test]
+fn applying_a_dynamic_mode_builds_its_engine() {
+    // apply_to_state assigned the mode field directly, so a config file (CLI
+    // --config, or an import in the TUI) asking for Scoreboard or Tomasulo got
+    // the in-order datapath instead, with no warning.
+    for mode in [PipelineMode::Scoreboard, PipelineMode::TomasuloRob] {
+        let cfg = PipelineConfig {
+            mode,
+            ..PipelineConfig::default()
+        };
+        let mut state = PipelineSimState::new();
+        cfg.apply_to_state(&mut state, &PipelineTiming::default());
+        assert_eq!(state.mode, mode);
+        assert!(state.ooo.is_some(), "{mode:?} has no engine");
+    }
+    let mut state = PipelineSimState::new();
+    PipelineConfig::default().apply_to_state(&mut state, &PipelineTiming::default());
+    assert!(state.ooo.is_none());
+}

tests/support/ui_pipeline_sim.rs

@@ -4102,9 +4102,6 @@ fn run_pipeline_mode_cached(
     state.set_mode(mode, &cpi);
     load_program_into_mem(asm, &mut cpu, &mut mem);
     state.reset_stages(0);
-    if let Some(engine) = state.ooo.as_mut() {
-        crate::capability::PipelineControl::reset(engine, 0);
-    }
     for _ in 0..4096 {
         pipeline_tick(&mut state, &mut cpu, &mut mem, &cpi, console);
         if state.halted || state.faulted || console.reading {
@@ -4257,9 +4254,6 @@ buf: .space 16
         state.set_mode(mode, &cpi);
         load_program_into_mem(asm, &mut cpu, &mut mem);
         state.reset_stages(0);
-        if let Some(engine) = state.ooo.as_mut() {
-            crate::capability::PipelineControl::reset(engine, 0);
-        }
         let mut tick = |cpu: &mut Cpu, console: &mut Console| {
             for _ in 0..512 {
                 pipeline_tick(&mut state, cpu, &mut mem, &cpi, console);
@@ -4354,9 +4348,6 @@ fn run_user_code_under_sv32(
     let mut state = PipelineSimState::new();
     state.set_mode(mode, &cpi);
     state.reset_stages(0);
-    if let Some(engine) = state.ooo.as_mut() {
-        crate::capability::PipelineControl::reset(engine, 0);
-    }
     for _ in 0..4096 {
         pipeline_tick(&mut state, &mut cpu, &mut mem, &cpi, &mut console);
         if state.halted || state.faulted {
@@ -4412,3 +4403,69 @@ fn a_fetch_page_fault_enters_the_handler_only_when_the_program_gets_there() {
         assert_eq!(cpu.mepc, 0x5000, "{mode:?}");
     }
 }
+
+#[test]
+fn reset_and_redirect_reach_the_out_of_order_engine() {
+    // reset_stages and redirect_pc used to touch only the in-order latches,
+    // so under Scoreboard or Tomasulo a reload, a reset or a PC edit left the
+    // engine fetching where the old program was.
+    let asm = r#"
+.text
+    li   a0, 1
+    li   a0, 2
+    li   a0, 3
+    halt
+"#;
+    for mode in [
+        crate::falcon::pipeline::PipelineMode::Scoreboard,
+        crate::falcon::pipeline::PipelineMode::TomasuloRob,
+    ] {
+        let mut console = Console::default();
+        let (mut cpu, mut state) = run_pipeline_mode_cached(asm, mode, &mut console);
+        assert!(state.halted, "{mode:?}");
+        let engine = |state: &PipelineSimState| {
+            let engine = state.ooo.as_ref().expect("a dynamic mode has an engine");
+            (
+                crate::capability::PipelineInspect::stats(engine).cycles,
+                engine.fetch_pc(),
+            )
+        };
+        assert_ne!(engine(&state).0, 0, "{mode:?}");
+
+        state.reset_stages(0);
+        assert_eq!(engine(&state), (0, 0), "{mode:?}: reset left the engine running");
+
+        state.redirect_pc(8);
+        assert_eq!(engine(&state).1, 8, "{mode:?}: redirect left the engine behind");
+
+        // And a fresh run from the reset really runs the program again.
+        let mut mem = CacheController::new(
+            CacheConfig::default(),
+            CacheConfig::default(),
+            vec![],
+            0x4000,
+        );
+        load_program_into_mem(asm, &mut cpu, &mut mem);
+        cpu.write(10, 0);
+        state.reset_stages(0);
+        let cpi = CpiConfig::default();
+        for _ in 0..256 {
+            pipeline_tick(&mut state, &mut cpu, &mut mem, &cpi, &mut console);
+            if state.halted || state.faulted {
+                break;
+            }
+        }
+        assert!(state.halted && !state.faulted, "{mode:?}");
+        assert_eq!(cpu.read(10), 3, "{mode:?}");
+    }
+}
+
+#[test]
+fn set_mode_builds_the_engine_even_if_mode_was_assigned_directly() {
+    let cpi = CpiConfig::default();
+    let mut state = PipelineSimState::new();
+    state.mode = crate::falcon::pipeline::PipelineMode::TomasuloRob;
+    assert!(state.ooo.is_none());
+    state.set_mode(crate::falcon::pipeline::PipelineMode::TomasuloRob, &cpi);
+    assert!(state.ooo.is_some());
+}