From 77eb4d7e8b4622f4e4459bea2085e706c30bad71 Mon Sep 17 00:00:00 2001
From: Adam Surgenor <Adam@Surgenor.me>
Date: Fri, 22 May 2026 10:25:20 +0100
Subject: [PATCH] [rtl] Remove redundant halt_if gating in DECODE state
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In the DECODE state of ibex_controller's FSM, halt_if is set in two
places when (enter_debug_mode || handle_irq) is true:

  1. An outer conditional gated by (stall || id_wb_pending).
  2. An inner conditional guarded by (!stall && !special_req && !id_wb_pending)
     that always sets halt_if = 1 when entering DBG_TAKEN_IF or IRQ_TAKEN.

The outer gating is logically redundant — the inner already sets
halt_if = 1 whenever (debug || irq) holds and the FSM transitions
through the non-special_req path. In the case the outer is meant
to cover (special_req high), the value of halt_if is observationally
irrelevant: special_req drives retain_id = 1 (controller.sv:577-581),
and id_in_ready_o is computed as

  assign id_in_ready_o = ~stall & ~halt_if & ~retain_id;

so retain_id forces id_in_ready_o low independent of halt_if. halt_if
has no other consumers (verified: not a module output).

The redundant gating introduced a long combinational arc on the
longest topological path:

  special_req → !special_req → AND with (stall||id_wb_pending) and
  (enter_debug_mode||handle_irq) → halt_if → id_in_ready_o → ctrl_fsm_ns

Consolidating halt_if to its simpler condition removes that arc.
The change is pure combinational restructuring (no new register,
no pipeline-stage change, no functional difference).

Measurements on a small-config Ibex (RV32IM, RV32MFast, RV32Zca,
WB=1, BP=1, no PMP/icache/security) synthesised with Yosys + ltp:

  critical_path_gates     119 → 108   (-9.24%)
  fmax estimate @ 0.2ns   42.0 → 46.3 MHz (+10.2%)
  num_cells (generic)     17,846 → 17,840 (-6)
  CoreMark 100i cycles    37,362,985 (unchanged)
  CoreMark CRCs           0xe714 / 0x1fd7 / 0x8e3a (golden)
  return42 / hello / fwd_test  all pass with identical cycle counts

Single block edit; total +13 / -12 lines in ibex_controller.sv.
---
 rtl/ibex_controller.sv | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/rtl/ibex_controller.sv b/rtl/ibex_controller.sv
index 1803c44d7c..7735a78a2a 100644
--- a/rtl/ibex_controller.sv
+++ b/rtl/ibex_controller.sv
@@ -607,9 +607,16 @@ module ibex_controller #(
           end
         end
 
-        // If entering debug mode or handling an IRQ the core needs to wait until any instruction in
-        // ID or WB has finished executing. Stall IF during that time.
-        if ((enter_debug_mode || handle_irq) && (stall || id_wb_pending)) begin
+        // cpu3/D9 T-008: hoist halt_if out from under special_req gating to
+        // shorten the LTP. The decode-time halt_if signal is consumed only by
+        // id_in_ready_o (= ~stall & ~halt_if & ~retain_id). When special_req
+        // is high retain_id is already 1, which forces id_in_ready_o low, so
+        // halt_if's exact value while special_req is high is observationally
+        // irrelevant. Decoupling halt_if from special_req here removes the
+        // special_req -> halt_if leg of the LTP, which arrives late because
+        // special_req depends on csr_pipe_flush -> csr_op_en -> instr_id_done.
+        // Combinational restructuring only; no new register, no new stage.
+        if (enter_debug_mode || handle_irq) begin
           halt_if = 1'b1;
         end
 
@@ -617,18 +624,12 @@ module ibex_controller #(
           if (enter_debug_mode) begin
             // enter debug mode
             ctrl_fsm_ns = DBG_TAKEN_IF;
-            // Halt IF only for now, ID will be flushed in DBG_TAKEN_IF as the
-            // ID state is needed for correct debug mode entry
-            halt_if     = 1'b1;
+            // halt_if is already asserted above for the (enter_debug_mode ||
+            // handle_irq) case.
           end else if (handle_irq) begin
             // handle interrupt (not in debug mode)
             ctrl_fsm_ns = IRQ_TAKEN;
-            // We are handling an interrupt (not in debug mode). Set halt_if to
-            // tell IF not to give us any more instructions before it redirects
-            // to the handler, but don't set flush_id: we must allow this
-            // instruction to complete (since it might have outstanding loads
-            // or stores).
-            halt_if     = 1'b1;
+            // halt_if already asserted above.
           end
         end